Skip to content

Update Newtonsoft.Json to 13.0.1#4167

Merged
zivkan merged 3 commits intodevfrom
dev-zivkan-update-newtonsoft.json
Aug 5, 2021
Merged

Update Newtonsoft.Json to 13.0.1#4167
zivkan merged 3 commits intodevfrom
dev-zivkan-update-newtonsoft.json

Conversation

@zivkan
Copy link
Member

@zivkan zivkan commented Jul 28, 2021

Bug

Fixes: NuGet/Home#11095

Regression? no

Description

Update package version, and do some cleanup.

PR Checklist

  • PR has a meaningful title

  • PR has a linked issue.

  • Described changes

  • Tests

    • Automated tests added
    • OR
    • Test exception
    • OR
    • N/A
  • Documentation

    • Documentation PR or issue filled
    • OR
    • N/A

@zivkan zivkan requested a review from a team as a code owner July 28, 2021 00:15
@zivkan
Copy link
Member Author

zivkan commented Aug 2, 2021

blocked on dotnet/sdk#19440

<IncludeInVSIX>true</IncludeInVSIX>
</Content>
<Content Include="$(SolutionPackagesFolder)newtonsoft.json\9.0.1\lib\net45\Newtonsoft.Json.dll">
<Content Include="$(SolutionPackagesFolder)newtonsoft.json\13.0.1\lib\net45\Newtonsoft.Json.dll">
Copy link
Member

Choose a reason for hiding this comment

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

nit: You can use the newtonsoft version variable.

Copy link
Member Author

Choose a reason for hiding this comment

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

The MSBuild property with the version number is defined in packages.targets which is imported after this line. I don't feel like spending time moving imports around to get this working without causing problems.

Copy link
Member

Choose a reason for hiding this comment

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

Content is an item, but the version number is a property.
Property pass happens before the items pass, so the ordering wouldn't matter.

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you sure properties are evaluated before items? I thought MSBuild was strictly "top down"

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll try to remember for the cleanup issue.

Copy link
Member

Choose a reason for hiding this comment

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

Only do that if you want to.
I don't want these comments to add any additional work.
It's nice to have, but really not that important in the grand scheme.

<Authenticode>3PartySHA2</Authenticode>
</FilesToSign>
<FilesToSign Include="$(SolutionPackagesFolder)newtonsoft.json\9.0.1\lib\net45\Newtonsoft.Json.dll">
<FilesToSign Include="$(SolutionPackagesFolder)newtonsoft.json\13.0.1\lib\net45\Newtonsoft.Json.dll">
Copy link
Member

Choose a reason for hiding this comment

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

nit: You can use the newtonsoft version variable.

Copy link
Member Author

Choose a reason for hiding this comment

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

This proj file doesn't currently import package.targets, and while it wouldn't be difficult to do so, I don't feel like spinning another build.

}
}

// temp: delete once the .NET SDK ships Newtonsoft.Json 13.0.1 or higher
Copy link
Member

Choose a reason for hiding this comment

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

tracking issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

NuGet/Home#11135

I'm not going to spin another build just to change the comment.

@zivkan zivkan merged commit 3c5cea9 into dev Aug 5, 2021
@zivkan zivkan deleted the dev-zivkan-update-newtonsoft.json branch August 5, 2021 21:23
heng-liu added a commit that referenced this pull request Aug 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update Newtonsoft.Json to 13.0.1

3 participants