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

Update MSAL's Newton.Json version to 13.0.1 #3521

Merged
merged 15 commits into from
Aug 8, 2022
Merged

Conversation

trwalke
Copy link
Member

@trwalke trwalke commented Jul 25, 2022

Fixes #
#3413

Changes proposed in this request
Updating newtonsoft.Json to 13.0.1.
Updating namespace
Applying Unity fixes
Applying Netstandard 2 fix
Applying UAP fix
Setting C# lang version to latest

Testing
Unity test app

Performance impact

Documentation

  • All relevant documentation is updated.

trwalke added 3 commits July 22, 2022 15:40
Updating namespace
Applying Unity fixes
Applying Netstandard 2 fix
Applying UAP fix
Setting C3 lang version to latest
suppressing warning
@trwalke trwalke linked an issue Jul 25, 2022 that may be closed by this pull request
2 tasks
@trwalke trwalke changed the title Trwalke/newton soft update Update MSAL's Newton.Json version to 13.0.1 Jul 25, 2022
@trwalke trwalke marked this pull request as ready for review July 25, 2022 23:01
Copy link
Contributor

@MichelZ MichelZ left a comment

Choose a reason for hiding this comment

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

Do you want to include non-code files like .snk, .csproj, pics, etc?

@bgavrilMS
Copy link
Member

Do you want to include non-code files like .snk, .csproj, pics, etc?

Not sure if there is any benefit for including those, since we're building the source code as part of MSAL.

@MichelZ
Copy link
Contributor

MichelZ commented Jul 26, 2022

Do you want to include non-code files like .snk, .csproj, pics, etc?

Not sure if there is any benefit for including those, since we're building the source code as part of MSAL.

Exactly. But they ARE included in this PR (I'd remove them)

Copy link
Contributor

@pmaytak pmaytak left a comment

Choose a reason for hiding this comment

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

Applying Unity fixes
Applying Netstandard 2 fix
Applying UAP fix

  • Can you specify what/where are the above specific changes? (Is it just a few flags?)
  • Maybe helpful to add a Readme in the top json folder with what the version of this Json.Net code is.
  • Are there any breaking changes in 13.x that affect us?

@bgavrilMS
Copy link
Member

Applying Unity fixes
Applying Netstandard 2 fix
Applying UAP fix

  • Can you specify what/where are the above specific changes? (Is it just a few flags?)
  • Maybe helpful to add a Readme in the top json folder with what the version of this Json.Net code is.
  • Are there any breaking changes in 13.x that affect us?

The Unity fix in particular should ideally be re-applied as a separate commit. It will help us track it down in the future.

@trwalke trwalke mentioned this pull request Jul 28, 2022
1 task
@trwalke
Copy link
Member Author

trwalke commented Jul 28, 2022

Applying Unity fixes
Applying Netstandard 2 fix
Applying UAP fix

  • Can you specify what/where are the above specific changes? (Is it just a few flags?)
  • Maybe helpful to add a Readme in the top json folder with what the version of this Json.Net code is.
  • Are there any breaking changes in 13.x that affect us?

The Unity fix in particular should ideally be re-applied as a separate commit. It will help us track it down in the future.

#3536

@bgavrilMS

@trwalke
Copy link
Member Author

trwalke commented Jul 29, 2022

Applying Unity fixes
Applying Netstandard 2 fix
Applying UAP fix

  • Can you specify what/where are the above specific changes? (Is it just a few flags?)
  • Maybe helpful to add a Readme in the top json folder with what the version of this Json.Net code is.
  • Are there any breaking changes in 13.x that affect us?

The net standard and UAP fixes are just flags. since the fixes are the same as we have in the main branch, they do not show in the diff and nuget wont let me comment on the lines where the are.

Added readme

no breaking changes that I have seen so far. They are not mentioned in the Release Notes

@pmaytak

# Conflicts:
#	src/client/Microsoft.Identity.Client/json/.editorconfig
@pmaytak
Copy link
Contributor

pmaytak commented Aug 3, 2022

UWP can be tested by building in Release and checking two properties in project properties - Optimize code and Compile with .NET Native tool chain:
image

* Applying Unity fixes to new Newtonsoft.JSON

* Making newtonsoft classes internal

* Add read me
Cleaned up unused files

* More classes moved to internal

Co-authored-by: trwalke <[email protected]>
@trwalke trwalke enabled auto-merge (squash) August 8, 2022 16:37
@trwalke trwalke merged commit 58d4b56 into main Aug 8, 2022
@trwalke trwalke deleted the trwalke/NewtonSoft_Update branch August 8, 2022 17:29
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.

[Bug] Upgrade Newtonsoft to the latest version
4 participants