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.

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

Add a JsonSerializerIsReflectionEnabledByDefault property #31626

Merged

Conversation

eiriktsarpalis
Copy link
Member

@eiriktsarpalis eiriktsarpalis commented Apr 5, 2023

Adds a msbuild property that ties to the feature switch implemented in dotnet/runtime#83844. Also disables the property when PublishAot=true.

Remove ASP.NET's feature switch and use JsonSerializerIsReflectionEnabledByDefault instead.

…ext.Json one.

Remove the defaulting of the System.Text.Json feature switch until a decision can be made when to turn it off by default.
@eerhardt eerhardt marked this pull request as ready for review April 13, 2023 13:36
@eerhardt eerhardt requested a review from vijayrkn as a code owner April 13, 2023 13:36
@eerhardt
Copy link
Member

I'm marking this PR as "ready for review".

I've removed the defaulting of the feature switch until a decision can be made in dotnet/runtime#84378.

I've removed the temporary ASP.NET JSON feature switch and am using the new one from System.Text.Json. This is needed to unblock ASP.NET work needed in preview4. So I want to get this merged without blocking on how to default the feature switch for all apps.

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

If this doesn't change the default value yet, this shouldn't impact the various MAUI workloads.

@eerhardt
Copy link
Member

I'm going to merge this by EOD today to unblock ASP.NET. Please let me know if you have any feedback.

Copy link
Member Author

@eiriktsarpalis eiriktsarpalis left a comment

Choose a reason for hiding this comment

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

Can't approve my own PR, but final changes LGTM.

@eerhardt
Copy link
Member

I needed to add back the ASP.NET feature switch until we can move the ASP.NET libraries off of it. Need to do this in a staged manner so we don't regress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants