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

Relax Json.NET version requirements #671

Closed
wants to merge 1 commit into from
Closed

Conversation

bennor
Copy link
Contributor

@bennor bennor commented Jun 6, 2019

This change downgrades the Newtonsoft.Json package reference (i.e. Json.NET) to a minimum of 9.01. We're not using any of the newer APIs, and all our tests pass, so I think this is okay. 🚀

It required adding an explicit dependency on System.Runtime.Serialization.Formatters to the netstandard 1.4 version, and in the process of adding that I cleaned up the 1.4 specific package references in the project a tiny bit.

My only concern here is the fairly large number of fixes in Json.NET between 9.0.1 and 12.0.2. This change won't stop stop anyone from referencing a later version though -- explicitly or otherwise. It just might open us up to some annoying bug reports. (As stated in the referenced issue, most people will be using a later version than 9.0.1 anyway.)

So this fixes #670, I think 😂

What kind of change does this PR introduce?

  • Downgrades the Newtonsoft.Json package reference to a minimum of 9.01

What is the current behavior?

What is the new behavior?

  • People using Azure Functions (or any other library with a strict requirement on a lower version than 12.0.1 but >= 9.0.1 will be able to use the latest version of Refit.

What might this PR break?

It could expose us to bug reports for any of the issues fixed in Json.NET since 9.0.1.

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features) Sort of. The old ones all run.
  • Docs have been added / updated (for bug fixes / features) Not really applicable.

This change downgrades the Newtonsoft.Json package reference to a minimum of 9.01. We're not using any of the newer APIs, and all our tests pass, so I think this is okay. 🚀

It required adding an explicit dependency on `System.Runtime.Serialization.Formatters` to the netstandard 1.4 version, and in the process of adding that I cleaned up the 1.4 specific package references in the project a tiny bit.

My only concern here is the [fairly large number of fixes in Json.NET between 9.0.1 and 12.0.2](https://github.com/JamesNK/Newtonsoft.Json/releases). This change won't stop stop anyone from referencing a later version though -- explicitly or otherwise. It just might open us up to some annoying bug reports. (As stated in the referenced issue, most people will be using a later version than 9.0.1 anyway.)

So this fixes #670, I think 😂
@bennor bennor requested a review from clairernovotny June 6, 2019 11:29
@clairernovotny
Copy link
Member

I have to say, I'm not keen on using minimum versions as I tend to think that lets bugs fester. I'd venture that a significant number of people only use the transitive dependencies and don't directly reference newer versions.

Is there a specific issue with the Azure Functions runtime tracking this? I thought they had been working on something so their runtime was isolated from user code.

@bennor
Copy link
Contributor Author

bennor commented Jun 6, 2019

Looks like there's a functions runtime issue for it. Can't tell for certain if it's being fixed.

There are about a hundred other ways AF does my head in, so in the grand scheme of things this is pretty minor. 🤣

@bennor
Copy link
Contributor Author

bennor commented Jun 6, 2019

@onovotny Is there any way to set up the package references so that people will be encouraged to use the latest version but still allow a previous version? Or is that just the default behaviour anyway?

If not I think we can abandon this PR and close the related issue.

@clairernovotny
Copy link
Member

@bennor not that I know of

@lock lock bot locked and limited conversation to collaborators Sep 6, 2019
@clairernovotny clairernovotny deleted the relax-json-net-reqs branch March 23, 2020 01:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Be less aggressive with dependency updates in core library?
2 participants