lautis / uglifier

Ruby wrapper for UglifyJS JavaScript compressor.
http://www.rubydoc.info/gems/uglifier
MIT License
613 stars 82 forks source link

Remove multi_json dependency #51

Closed sferik closed 11 years ago

sferik commented 11 years ago

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 a similar pull request to execjs and will shortly be submitting pull requests to sprockets and 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.

lautis commented 11 years ago

I have some doubts about dropping Ruby 1.8 as it could be enabled by installing json gem. Maybe it's just time to leave Ruby 1.8.7 behind, it was fun while it lasted.

This is clearly breaking change for Ruby 1.8.7 users, but I'm not totally sure how semantic versioning should apply here as the "public API" doesn't change at all. Incompatible versions are just ignored by older Rubies. Still, might be worth doing a 3.0 to avoid confusion.

sferik commented 11 years ago

If you’re reluctant to bump to 3.0, I’d be willing to modify this patch to swap the multi_json dependency for a json gem dependency. Just say the word.

sferik commented 11 years ago

I’ve reworked this patch to be completely Ruby 1.8 compatible. Would you be willing to take another look?