search

domcleal / spdx-licenses

SPDX license and identifier lookup
MIT License
14 stars 12 forks source link

Removed json gem dependency #4

Closed andrew closed 8 years ago

andrew commented 8 years ago

Not needed since ruby 1.9+ as it's including in the standard library

Similar pr sent to rails recently: https://github.com/rails/rails/pull/23453

domcleal commented 8 years ago

Thanks, would you mind adding gem 'json', :platform => :ruby_18 to the Gemfile to ensure tests pass? You'll want to rebase on master first as I've fixed the tests that are currently failing.

andrew commented 8 years ago

@domcleal adding that to the gem file won't actually fix it for people wanting to use ruby 1.8 though, if you want to fully support 1.8, which has been unsupported for nearly three years now, then don't merge this pr as it will fail to work for 1.8 users.

domcleal commented 8 years ago

Yes, I'm happy for Ruby 1.8.7 users to add their own dependencies on the json gem if they want to use this, but I'd like to ensure the gem still runs on 1.8.7. The Gemfile update is just needed for tests on 1.8.7 for spdx-licenses itself.

Though it's long since unsupported upstream, it's still common in certain places - this gem is used in the Puppet community, and 1.8's often still tested.

andrew commented 8 years ago

@domcleal cool, updated and rebased 👍

domcleal commented 8 years ago

Merged, thanks @andrew!