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

Revert: Improve MSBuild SDK resolver by parsing global.json with System.Runtime.Serialization.Json #3157 #3203

Merged
merged 2 commits into from
Jan 24, 2020

Conversation

jeffkl
Copy link
Contributor

@jeffkl jeffkl commented Jan 24, 2020

Bug

Fixes: NuGet/Home#9098
Regression: Yes

  • Last working version:
  • How are we preventing it in future:

Fix

Details: Reverted #3157 and added unit tests to make sure this doesn't regress again.

Testing/Validation

Tests Added: Yes
Reason for not adding tests:
Validation:

@jeffkl jeffkl changed the title Revert globaljson reader change Improve MSBuild SDK resolver by parsing global.json with System.Runtime.Serialization.Json #3157 Jan 24, 2020
Copy link
Contributor

@dtivel dtivel left a comment

Choose a reason for hiding this comment

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

Thanks for adding tests. That's great.

Don't forget to update the PR title to accurately reflect the PR's intent.

@jeffkl jeffkl changed the title Improve MSBuild SDK resolver by parsing global.json with System.Runtime.Serialization.Json #3157 Revert: Improve MSBuild SDK resolver by parsing global.json with System.Runtime.Serialization.Json #3157 Jan 24, 2020
Copy link
Member

@nkolev92 nkolev92 left a comment

Choose a reason for hiding this comment

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

You can consider keeping your test environment changes :)
Specifically your tests could still be using NuGet's infra.

@jeffkl
Copy link
Contributor Author

jeffkl commented Jan 24, 2020

You can consider keeping your test environment changes

I will send a follow up PR to remove the TestEnvironment class as a separate commit to expedite getting this in the next build.

@nkolev92
Copy link
Member

Sounds good.
Feel free to merge when green, but Monday is also fine afaik.

@jeffkl jeffkl merged commit 2e063a2 into NuGet:dev Jan 24, 2020
@jeffkl jeffkl deleted the revert-globaljson-reader-change branch September 8, 2021 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants