Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Binder gen] To align with reflection impl, update property binding logic that handles missing config values #86333

Closed
layomia opened this issue May 16, 2023 · 5 comments
Closed

[Binder gen] To align with reflection impl, update property binding logic that handles missing config values #86333

layomia opened this issue May 16, 2023 · 5 comments
Assignees
@steveharter
Labels
area-Extensions-Configuration blocking-release bug Priority:1 Work that is critical for the release, but we could probably ship without
Milestone
8.0.0

Comments

@layomia
Copy link
Contributor

layomia commented May 16, 2023 •
edited
Loading

Background: #86285 (comment). This is for consistency with the reflection binder.
@layomia layomia added this to the 8.0.0 milestone May 16, 2023
@layomia layomia self-assigned this May 16, 2023
@ghost
Copy link

ghost commented May 16, 2023

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

Issue Details

Background: #86285 (comment).

Author: layomia
Assignees: layomia
Labels:

area-Extensions-Configuration
Milestone: 8.0.0

@layomia layomia changed the title Config binder source-get: ensure property setters are called even when there's no matching config value. Config binder source-gen: ensure property setters are called even when there's no matching config value. May 16, 2023
This was referenced May 16, 2023
Merged
Closed
@layomia layomia added the bug label May 26, 2023
@layomia
Copy link
Contributor Author

layomia commented Jul 21, 2023

Per feedback from @tarekgh all binding differences from the reflection implementation need to be addressed before we ship; this includes differences in exception handling. Marking as blocking release.

Divergences are reflected in ConditionalFact applications in the source gen tests.

@ericstj
Copy link
Member

ericstj commented Aug 15, 2023

Triage: users may notice this difference. We have had multiple reports of this when we regressed in the runtime implementation.

@layomia layomia changed the title Config binder source-gen: ensure property setters are called even when there's no matching config value. Update config gen property setting when there's no matching config value to match reflection impl Sep 1, 2023
@layomia
Copy link
Contributor Author

layomia commented Sep 1, 2023

Scenarios

@layomia layomia mentioned this issue Sep 1, 2023
Closed
@ericstj ericstj added the Priority:1 Work that is critical for the release, but we could probably ship without label Sep 5, 2023
@layomia layomia changed the title Update config gen property setting when there's no matching config value to match reflection impl [Binder gen] To align with reflection impl, update property binding logic that handles missing config values Sep 8, 2023
@steveharter steveharter mentioned this issue Sep 13, 2023
Merged
@ericstj
Copy link
Member

ericstj commented Sep 20, 2023

Fixed in RC2 with #92167

@ericstj ericstj closed this as completed Sep 20, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Oct 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@steveharter steveharter

Labels
area-Extensions-Configuration blocking-release bug Priority:1 Work that is critical for the release, but we could probably ship without
Projects
None yet
Milestone
8.0.0
Development

No branches or pull requests
3 participants
@steveharter @ericstj @layomia