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

Removing JSON for other ruby #8047

Merged
merged 1 commit into from Oct 28, 2012

Conversation

Projects
None yet
5 participants
Member

arunagw commented Oct 28, 2012

It's only required for 1.8 and for JRuby!
I was fixing a bug with JSON pure on 1.8.7
and I found that we have included json for other
1.8 > rubies also.

@arunagw arunagw Removing JSON for other ruby
It's only required for 1.8 and for JRuby!
I was fixing a bug with JSON pure on 1.8.7
and I found that we have included json for other
1.8 > rubies also.
1adb784

Hey mate, not sure I got what you are trying to explain. We don't need the json gem for Ruby 1.9 you say?

Member

steveklabnik commented Oct 28, 2012

Yes, you do not:

steve at thoth in ~
$ irb
requi1.9.3p194 :001 > require 'json'
 => true 
1.9.3p194 :002 > RUBY_VERSION
 => "1.9.3" 

So the question is: should we stick with the impl that comes with ruby, or use the gem that can be updated if necessary?

Member

arunagw commented Oct 28, 2012

I think this is question that we need to decide. I can confirm in a while if tests are running fine.

What you say?

Member

arunagw commented Oct 28, 2012

All Green on this branch.

Member

steveklabnik commented Oct 28, 2012

IIRC, @evanphx explained to me that the json gem does the right thing nowadays with all rubies; I took multi_json out of resque because of this. I don't think that we need anything specific, and if we need to 'update if necessary' we can just add it back then.

Owner

tenderlove commented Oct 28, 2012

@carlosantoniodasilva the JSON gem ships with ruby, people can still update the gem after installing ruby.

It should be safe to apply this patch.

Owner

rafaelfranca commented Oct 28, 2012

Yes, it should be fine. But we will have json installed because rdoc and w3c_validators have it as dependency.

@rafaelfranca yes, I noticed that.

@tenderlove ok, thanks for clarifying.

@carlosantoniodasilva carlosantoniodasilva added a commit that referenced this pull request Oct 28, 2012

@carlosantoniodasilva carlosantoniodasilva Merge pull request #8047 from arunagw/json_only_in_18
Removing JSON for other ruby
2b5d3aa

@carlosantoniodasilva carlosantoniodasilva merged commit 2b5d3aa into rails:3-2-stable Oct 28, 2012

@carlosantoniodasilva carlosantoniodasilva added a commit that referenced this pull request Oct 28, 2012

@carlosantoniodasilva carlosantoniodasilva Merge pull request #8047 from arunagw/json_only_in_18
Removing JSON for other ruby
a698175
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment