Skip to content
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

Namespace inferred serializer resolution. #60

Merged
merged 1 commit into from
Jul 24, 2016

Conversation

drn
Copy link
Member

@drn drn commented Jul 22, 2016

This reimplements the changes in pull #58 on top of the refactor of pull #59.

This borrows some logic from the serializer resolution logic in active_model_serializers. Given that active_model_serializers no longer supports the namespace option passed into the serializer_for method, in order to support namespace inference we need to reuse some of their serializer_for logic in grape-asm. I will work with the asm team to add back support for that option, but in order to support the existing 0.10.x asm versions, we need to reimplement some of that logic.

@@ -24,5 +24,5 @@ Gem::Specification.new do |gem|
gem.add_development_dependency 'rack-test'
gem.add_development_dependency 'rake'
gem.add_development_dependency 'guard-rspec'
gem.add_development_dependency 'rubocop', '0.28.0'
gem.add_development_dependency 'rubocop'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cause problems. You can move it out of gemspec and into the Gemfile, but you want a version or in the future bundle install brings a newer one and suddenly things fail.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I kept it in the .gemspec but locked the version here. Pushed it straight to master, as it's a small change

@dblock
Copy link
Member

dblock commented Jul 24, 2016

Merging. See my comment on Rubocop, can address later.

@dblock dblock merged commit eb43f68 into ruby-grape:master Jul 24, 2016
@drn drn deleted the namespace-inference branch July 24, 2016 03:15
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.

2 participants