collectiveidea / json_spec

Easily handle JSON in RSpec and Cucumber
rubygems.org/gems/json_spec
MIT License
919 stars 114 forks source link

Update for deprecations in multi_json 1.3.0 #26

Closed lewinski closed 12 years ago

lewinski commented 12 years ago

This cleans up some deprecation warnings when using json_spec with multi_json >= 1.3.0. It feels better to bump the multi_json dependency rather than put in begin/rescue blocks to handle fallback to the older methods if the new code doesn't work, so the gemspec was also updated.

lewinski commented 12 years ago

After reviewing what Rails is doing about this for the 3.2 branch, I'm not sure the original changes I posted are the most appropriate. I am going to resubmit code that is compatible with the other change.

https://github.com/rails/rails/pull/5861

zmoazeni commented 12 years ago

@lewinski thanks for the pull request. I decided to go ahead and just use the newer MultiJson method as of https://github.com/collectiveidea/json_spec/commit/9d72cdb9d1f051951563cbe0c1f3372795ce3dfa

lewinski commented 12 years ago

The concern I have with doing that is that json_spec 1.0.1 is not going to be compatible with the next rails 3.2 release, whenever that is. That seems like a fairly big breaking change that doesn't belong in a point release.

zmoazeni commented 12 years ago

Looks like someone changed their mind after that pull request was merged in and allows ~> 1.3 https://github.com/rails/rails/commit/625f4b9199df47a40d1ffde6603e01cc753d06e0#activesupport/activesupport.gemspec and https://github.com/rails/rails/commits/master/activesupport/activesupport.gemspec

I'll let @laserlemon decide if he wants to keep the dependency to ~> 1.3 or allow newer.

lewinski commented 12 years ago

Yeah, that's on the master (4.0) branch though. See the following for 3.2:

https://github.com/rails/rails/blob/3-2-stable/activesupport/activesupport.gemspec

zmoazeni commented 12 years ago

Ah good catch. I thought I was looking at the 3.2 branch. Well that's uncool. I'll bring it up with the MultiJson guys.

zmoazeni commented 12 years ago

I'm keeping an eye on https://github.com/rails/rails/pull/5896 - if that gets merged in, we will be alright sticking with >= 1.3

zmoazeni commented 12 years ago

https://github.com/rails/rails/pull/5896 was pulled in to allow ~> 1.0 - httparty decided to reuse the method switching and keep ~> 1.0 https://github.com/jnunemaker/httparty/commit/1ec90d1f85a1b71c4dcb603235ce21d7fe65daf7

I'm thinking changing the gemspec to do ~> 1.3 maybe better. We'll stay compatible while not having to do the load/decode method switching.

zmoazeni commented 12 years ago

@laserlemon fixed this as of c2c3780ec2dddce965d197785149ecb2570245f8

laserlemon commented 12 years ago

Considering json_spec is a testing gem, I want to make sure its dependencies are as open as they can be. The feature-detection nature of the fix isn't all that pretty but… oh well. Thanks for all the help guys. :clap: