-
Notifications
You must be signed in to change notification settings - Fork 325
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
Upgrade to Newtonsoft.Json 13.0.1 #3815
Conversation
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 cannot see coverage but I am unsure all the new ifs added are covered.
samples/Microsoft.TestPlatform.Protocol/Microsoft.TestPlatform.Protocol.csproj
Show resolved
Hide resolved
src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestObjectConverter.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestObjectConverter.cs
Outdated
Show resolved
Hide resolved
...tPlatform.CrossPlatEngine/AttachmentsProcessing/DataCollectorAttachmentProcessorAppDomain.cs
Outdated
Show resolved
Hide resolved
scripts/build.sh
Outdated
@@ -436,7 +436,7 @@ function publish_package() | |||
done | |||
#*************************************************************************************************************# | |||
|
|||
newtonsoft=$TP_PACKAGES_DIR/newtonsoft.json/9.0.1/lib/netstandard1.0/Newtonsoft.Json.dll | |||
newtonsoft=$TP_PACKAGES_DIR/newtonsoft.json/13.0.1/lib/netstandard1.0/Newtonsoft.Json.dll |
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.
Since we're reading the version from Versions.props in build.ps1, can we do it here as well? This file has samples.
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.
done.
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.
You said done but I still see the version used 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.
Hoping that all that return null
are not breaking something(if for some reason we're accidentally relying on some try/catch for NRE) 🤞
scripts/build.sh
Outdated
@@ -436,7 +436,7 @@ function publish_package() | |||
done | |||
#*************************************************************************************************************# | |||
|
|||
newtonsoft=$TP_PACKAGES_DIR/newtonsoft.json/9.0.1/lib/netstandard1.0/Newtonsoft.Json.dll | |||
newtonsoft=$TP_PACKAGES_DIR/newtonsoft.json/13.0.1/lib/netstandard1.0/Newtonsoft.Json.dll |
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.
You said done but I still see the version used here.
src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestObjectConverter.cs
Outdated
Show resolved
Hide resolved
* Upgrade to Newtonsoft.Json 13.0.1 (#3815) * Update versions * Move version to 17.3.3
* Fix build * Upgrade to Newtonsoft.Json 13.0.1 (#3815) * Upgrade arcade * update vs image used to build * Add 3.1.32 dotnet runtime * Ignore acceptance test for multi targetting, we did not touch that, it relies on an edge case, and it does not depend on newtonsoft * Bump version * Fix version
* Upgrade to Newtonsoft.Json 13.0.1 (#3815) * upgrade arcade * Fix * downgrade ubuntu * Dowgrade ubuntu more * Bump version * Fix version
Upgrade to Newtonsoft.Json 13.0.1.