Skip to content
This repository has been archived by the owner on Jun 10, 2018. It is now read-only.

Remove multi_json dependency #440

Merged
merged 2 commits into from
Aug 14, 2013

Conversation

sferik
Copy link
Contributor

@sferik sferik commented May 12, 2013

As you are probably aware, Ruby 1.8 will stop being updated—even for critical security issues—past June. This patch changes the minimum Ruby version to 1.9, as has already been done in the forthcoming version of Rails, currently in release candidate.

Since Ruby 1.9 includes json in the standard library, multi_json is no longer necessary. I am a longtime maintainer of multi_json but will stop supporting the library in June.

This patch replaces multi_json with stdlib json. It will use the json gem if a newer version is available.

I have already submitted similar pull requests to execjs and uglifier and will shortly be submitting a pull request to rails itself, so multi_json need not be a dependency of Rails 4 applications. This should have the effect of making JSON parsing and generation somewhat faster by removing an abstraction layer that is no longer necessary.

@@ -12,7 +12,6 @@ Gem::Specification.new do |s|
s.executables = ["sprockets"]

s.add_dependency "hike", "~> 1.2"
s.add_dependency "multi_json", "~> 1.0"
Copy link

Choose a reason for hiding this comment

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

What about adding json gem as a dependency instead? Without it buggy sdtlib version might get invilved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure the json gem should be a dependency. If users are experiencing bugs in the stdlib version of json, they are free to add the json gem to their applications, but this is not necessarily a concern of this library. All the tests pass without requiring the json gem.

@josh
Copy link
Contributor

josh commented May 13, 2013

Sprockets 2.x will continue to support 1.8.

I'll drop support in 3.x.

@sferik
Copy link
Contributor Author

sferik commented May 13, 2013

@josh Do you have any sense of when version 3 will be released? Is it safe to assume 2.9 will be the last release in the 2.x series or are you planning to ship 2.10 before 3.0? Is there a v3 branch, to which I can submit this pull request? Or will you just wait to merge this until you start working on 3.0 in master?

@schmurfy
Copy link

even when ruby 1.8 support is dropped I don't see any reason to remove multi_json, the stdlib is for a large part nothing more than a museum, we should always prefer a gem over this...
If I have say oj in my Gemfile I prefer it to be used instead instead of an old version of the json gem.

@josh josh merged commit 8e7d744 into sstephenson:master Aug 14, 2013
@sferik sferik deleted the remove_multi_json_dependency branch August 14, 2013 21:57
@schmurfy
Copy link

is the integrated json above say oj ? because the last I check I did not get this idea at all...
Why forcing using one when there was already everything to use the best one available ?

Just my two cents (again).

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.

4 participants