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

Remove support for alternate JSON backends #339

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

iainbeeston
Copy link
Contributor

@iainbeeston iainbeeston commented Jun 29, 2016

I want to resurrect #206 , which removed support for alternative JSON backends. I believe that rather than putting this off to a version 3.0 release which will probably never come, we should merge this in now, in a way that is as backwards compatible as possible. I believe it could be done in a minor release, so long as we keep the external methods related to setting the json backend.

The one catch here is that Ruby 1.8 doesn't come with the json gem, and it would need installing by default... but in reality json-schema hasn't worked with ruby 1.8 for some time.

@iainbeeston iainbeeston force-pushed the drop-json-backend-support branch 6 times, most recently from 7aa9df1 to 31eddae Compare June 30, 2016 14:41
@RST-J
Copy link
Contributor

RST-J commented Jul 20, 2016

I personally do not have a problem with that, but I'm not sure whether is consistent to remove alternative JSON backend support in a minor version. But if I see it correctly, other backends can still be used, only the way how to achieve this changes (enabling compatibility mode instead of setting a multi-json backend).
And I see another argument for doing this: Currently, two installations of a project can behave differently depending solely on the fact that in one environment the multi-json gem is installed and in the other not.
So I'd say, we should do it. 👍
@pd or @hoxworth any comments on this?

@iainbeeston
Copy link
Contributor Author

I agree, it's not clear whether this is a breaking change (and therefore whether it would need a major release).

My argument is that this can be released in a minor version, because so long as you're using ruby 1.9+ (and json-schema has major bugs on ruby 1.8) you do not need to update your code to use this branch. Even if you were previously using multijson or oj or yajl or whatever, json-schema would silently fall back to using the standard json gem. You might get better performance by making yajl or oj use compatibility mode, but nothing will break. What's more, if you are explicitly setting the json backend, we show a deprecation warning, which I hope will make it obvious to anyone who does not read the changelog when upgrading.

@iainbeeston
Copy link
Contributor Author

It's been 3 months since this was raised... Any thoughts?

@iainbeeston iainbeeston force-pushed the drop-json-backend-support branch 2 times, most recently from b9ac1e2 to b5854f3 Compare September 29, 2016 14:14
@RST-J
Copy link
Contributor

RST-J commented Oct 4, 2016

To me it is fine to merge this. If it - possibly - only breaks things that have been broken before there is no point in deferring this.

@RST-J
Copy link
Contributor

RST-J commented Nov 8, 2016

So, do we merge this?

@iainbeeston
Copy link
Contributor Author

@RST-J I think we should merge this (after I rebase, of course)

@leoarnold
Copy link

@RST-J @iainbeeston Are you still on this? MultiJSON is causing an intricate headache: ohler55/oj#441 (comment)

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.

4 participants