Skip to content

Loosens ActiveSupport to >= 3.0.0. Fixes #289 #290

New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

wwahammy
Copy link
Contributor

@wwahammy wwahammy commented Jan 8, 2018

All the tests pass. bundle exec rake has some rubocop reports but they're not related to the change.

@coveralls
Copy link

coveralls commented Jan 8, 2018

Coverage Status

Coverage remained the same at 92.74% when pulling 3c3c5c6 on ericschultz:loosen_activesupport_to_3 into 064ecfb on ruby-grape:master.

@LeFnord
Copy link
Member

LeFnord commented Jan 11, 2018

I tested it with 'activesupport', '~>3.0' → uses 3.2.22.5, there one error occurs …

 1) Grape::Entity except option for nested entity
     Failure/Error: Address.represent c[:address], Grape::Entity::Options.new(o.opts_hash.except(:full))
     
     NoMethodError:
       undefined method `except' for {:attr_path=>[:address]}:Hash
     # ./spec/grape_entity/hash_spec.rb:19:in `block in <class:Company>'
     ...
     # ./lib/grape_entity/entity.rb:490:in `serializable_hash'
     # ./spec/grape_entity/hash_spec.rb:36:in `block (2 levels) in <top (required)>'

it seems, that if the people will really use version 3.x of it, then it didn't work correct

@LeFnord
Copy link
Member

LeFnord commented Jan 11, 2018

… so I'll close it

@LeFnord LeFnord closed this Jan 11, 2018
@wwahammy
Copy link
Contributor Author

Sorry about that.

Just looking into this, the problem seems to be that activesupport's extensions for the hash class aren't being included in the spec suite. When I add require 'active_support/core_ext/hash' to the top of spec_helper.rb, the specs pass on 'activesupport', '~>3.0' I'm not sure on the best way to address this (I'm still a bit of a beginner to Ruby).

I'm happy to correct the problem if you have any suggestions.

@LeFnord
Copy link
Member

LeFnord commented Jan 11, 2018

ok, then it is so, I'll re-open the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants