-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Clean up STJ references Fixes #39902 #40645
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
Changes from all commits
9763f0b
04482a8
9fe4889
3cf8751
2d84bf5
bf21493
091f0b4
1a2fee1
337965f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,7 +28,7 @@ | |
| <ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'"> | ||
| <!-- Netfx version builds against the lowest version of System.Text.Json that's guaranteed to be shipped with MSBuild in VS --> | ||
| <PackageReference Include="Microsoft.Bcl.AsyncInterfaces" VersionOverride="8.0.0" /> | ||
|
Comment on lines
29
to
30
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not needed?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was confused in looking at the 8.0.4xx-targeted version, as this wasn't needed then...but that's because it was using Newtonsoft.Json instead. Apparently it was fully switched to STJ, so now it became a problem. Thanks for catching that 🙂 |
||
| <PackageReference Include="System.Text.Json" VersionOverride="8.0.3" /> | ||
| <PackageReference Include="System.Text.Json" VersionOverride="$(SystemTextJsonToolsetPackageVersion)" /> | ||
| <Reference Include="System" /> | ||
| <Reference Include="System.Core" /> | ||
| </ItemGroup> | ||
|
|
||
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.
Probably want a review from @tmat but with CPM now enabled, this may not be required anymore like it was before.
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.
Having an explicit package reference is no longer required, or removing it is no longer required?
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.
Having the explicit reference is no longer required, I think. If I understand the original issue per the comment, a transitive version was picked up that didn't work with source build and adding the explicit reference up-versioned to one that was supported. CPM should do the version change as well.