Skip to content

Clarify chained configuration TryGet semantics and apply review-thread fixes - #131577

Closed
rosebyte with Copilot wants to merge 2 commits into
mainfrom
copilot/fix-comments-in-review-thread-again
Closed

Clarify chained configuration TryGet semantics and apply review-thread fixes#131577
rosebyte with Copilot wants to merge 2 commits into
mainfrom
copilot/fix-comments-in-review-thread-again

Conversation

Copilot AI commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

main PR

Description

This updates only the code requested in review thread #pullrequestreview-4799878306: clarify ChainedConfigurationProvider.TryGet semantics and align tests with nullable contracts.

  • Provider behavior + API docs

    • TryGet now treats any non-null value as present (including empty string).
    • XML docs now explicitly state non-null semantics.
  • Tests aligned with contract

    • Added focused coverage for:
      • empty string value returns true
      • missing key returns false and null
    • TryGet out usage is nullable (string?) to match IConfigurationProvider.TryGet.
public bool TryGet(string key, out string? value)
{
    value = _config[key];
    return value is not null;
}

Customer Impact

Without this change, chained configuration can incorrectly treat empty string as missing, allowing lower-priority values to win unexpectedly. API docs also remain ambiguous for consumers interpreting TryGet.

Regression

This is a behavioral correctness fix for chained provider precedence with empty-string values; null remains treated as absence.

Testing

Targeted unit tests were added for TryGet empty-string and missing-key behavior, matching the intended nullable contract.

Risk

Low. The functional change is a single-condition update in TryGet, with scoped tests covering the affected behavior.

Package authoring no longer needed in .NET 9

IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version.
Keep in mind that we still need package authoring in .NET 8 and older versions.

Copilot AI review requested due to automatic review settings July 30, 2026 08:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
16 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Co-authored-by: rosebyte <14963300+rosebyte@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 30, 2026 09:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot AI changed the title [WIP] Fix code based on review comments Clarify chained configuration TryGet semantics and apply review-thread fixes Jul 30, 2026
Copilot AI requested a review from rosebyte July 30, 2026 09:26
@rosebyte rosebyte closed this Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants