Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Multi json fix 3 2 stable #5861

Merged
merged 1 commit into from Apr 16, 2012

Conversation

Projects
None yet
4 participants
Member

arunagw commented Apr 16, 2012

It's using new API now.

Owner

jeremy commented Apr 16, 2012

This forces apps to upgrade multijson to 1.3.0+ and limits future compat to 1.3.x

It'd be cleaner to restrict it to >= 1.0, < 1.3 and keep using the same API

Member

arunagw commented Apr 16, 2012

@jeremy PR updated.

thanks.

@arunagw arunagw multi_json is restricted to < 1.3
Some API changes are there above 1.3. 
3-2-stable
9b14e3f

@jeremy jeremy added a commit that referenced this pull request Apr 16, 2012

@jeremy jeremy Merge pull request #5861 from arunagw/multi_json_fix_3-2-stable
Restrict multi_json to >= 1.0, < 1.3 to avoid API changes in 1.3
9a97699

@jeremy jeremy merged commit 9a97699 into rails:3-2-stable Apr 16, 2012

Contributor

sferik commented Apr 19, 2012

For the record, MultiJson 1.3 is intended to be completely backwards compatible with all the previous 1.x releases, following Semantic Versioning. It throws deprecation warnings about APIs that will be removed come version 2, but it should not break any of those APIs.

As far as I can tell, there's no good reason to restrict people to < 1.3. If they want to use MultiJson 1.3 with Rails 3.2, they'll get warnings, which they can either disable or ignore. The other option is for them to stick with an earlier version of MultiJson, which should work just fine, but it seems like this decision should be up to the user instead of being enforced by Active Support.

Owner

jeremy commented Apr 19, 2012

@sferik Cool, seems legit if it's on the way out in 2.0. However, it's similarly annoying to force deprecation warnings on everyone who installs Rails, considering it's something they can't do anything about! To work around, we can allow any 1.x but feature-detect which API to use.

@jeremy jeremy added a commit that referenced this pull request Apr 21, 2012

@jeremy jeremy Merge pull request #5896 from sferik/revert_5861
Revert #5861. Feature-detect which MultiJson API to use.
c3d50b3
Contributor

maletor commented Aug 2, 2012

@jeremy Feature detect is the way to go here. The latest version of uglifier depends on ~> 1.3.

@AlexRiedler AlexRiedler pushed a commit to AlexRiedler/rails that referenced this pull request Jan 9, 2013

@jeremy jeremy + Alex Riedler Merge pull request #5896 from sferik/revert_5861
Revert #5861. Feature-detect which MultiJson API to use.
Conflicts:
	activesupport/activesupport.gemspec

This backports multi_json version depedency changes as applied.

Rationale: #5861

Patch by sferik
7b9bab6

@rafaelfranca rafaelfranca added a commit that referenced this pull request Jan 9, 2013

@rafaelfranca rafaelfranca Merge pull request #8846 from AlexRiedler/revert_5861
Backport multi_json dependency revert of #5861 to 3-1-stable
b816e8e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment