-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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.
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
[release/7.0] Fix Configuration to ensure calling the property setters. #80562
Conversation
Tagging subscribers to this area: @dotnet/area-extensions-configuration Issue DetailsBackport of #80438 to release/7.0 /cc @tarekgh Customer ImpactTestingRiskIMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.
|
I am going to add the package authoring change here. |
@@ -5,8 +5,8 @@ | |||
<EnableDefaultItems>true</EnableDefaultItems> | |||
<IsPackable>true</IsPackable> | |||
<EnableAOTAnalyzer>true</EnableAOTAnalyzer> | |||
<ServicingVersion>2</ServicingVersion> | |||
<GeneratePackageOnBuild>false</GeneratePackageOnBuild> | |||
<ServicingVersion>3</ServicingVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ViktorHofer @carlossanlop @ericstj could you please help review this package authoring change in this file? Just to ensure I am doing the right thing here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
CI failures unrelated. I opened an issue, I'm seeing some other PRs affected by this: #80578 |
This is approved offline by the email. |
Approved by Tactics via email (7.0.3). |
Backport of #80438 to release/7.0
/cc @tarekgh
Customer Impact
Apps which define a configuration and expect to bind to app types, in some cases property setters of such type get skipped (not called) and causing the types not initialized correctly. This causes the apps to see unexpected behaviors when using the objects created by the configuration. This is a regression caused by some changes done in .NET 7.0 refactoring and support of new scenarios.
Testing
I spent enough time manually testing all scenarios related to this change and I have added more regression tests to avoid running into such regressions again. I have successfully run all regression tests too.
Risk
Very low, the change is a quite simple and scoped fix which shouldn't affect anything else more than ensuring calling the property setters during the binding.
IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.