Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Update System.Text.Json to 5.0.2 #6784
Update System.Text.Json to 5.0.2 #6784
Changes from all commits
664fdd6
7a8ef0f
aa2dd51
b70bb36
49818ae
c0b9875
44777a1
edeed7f
e16b4a0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
This file was deleted.
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.
Should we also add binding redirects for System.ValueTuple as we did for VS so that a 4.0.3.0 binding can load the 4.0.0.0 one from the GAC?
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.
@rainersigwald Thoughts?
I'm tempted to leave as-is. The binding redirects in VS were really just for tests ...
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.
Counter point: For consistency of devenv.exe and msbuild.exe ... it might make sense to have the same redirects.
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.
No, the binding redirects in VS were not just for tests. Folks can reference 4.0.0.0 or 4.0.3.0 during compilation (among others). The CLR will not bind 4.0.3.0 to 4.0.0.0 (or 4.0.0.0 to 4.0.3.0) without binding redirects. VS must have redirects or else our very diverse codebase won't always run together. MSBuild tasks come from 3rd parties as well so IMO it behooves msbuild to have binding redirects for all the assemblies it ships (or doesn't by relying on the GAC) as well.
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.
I think it makes sense to have the binding redirect (we're trying to be consistent about that, see #6334) and also for it to be the same as VS's (down to 4.0.0.0).
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.
Pushed a change to do this (and pull it from our VSIX).
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.
(I meant they were just to get tests to pass since the runner couldn't pull in newer) but you're right! Sounds good!