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

Meta no longer handled in Base adapter. #1695

Merged
merged 1 commit into from
Apr 21, 2016
Merged

Conversation

beauby
Copy link
Contributor

@beauby beauby commented Apr 20, 2016

Make each relevant adapter (i.e. Json and JsonApi) handle top level meta.

hash = serializable_hash(options)
include_meta(hash)
hash
serializable_hash(options)
end
Copy link
Member

Choose a reason for hiding this comment

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

alias method should be fine here, unless we want to keep the root option here like in the am:s:j in rails

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It failed because the alias would be bound to the pre-inheritance placeholder method.

@beauby
Copy link
Contributor Author

beauby commented Apr 20, 2016

Tests failing because of cyclomatic complexity of JsonApi#success_document.

@NullVoxPopuli
Copy link
Contributor

@beauby wow, yeah -- it doesn't even look that bad.

As a whole, LGTM, nice work

@beauby
Copy link
Contributor Author

beauby commented Apr 20, 2016

Now Appveyor is failing for no reason.

@beauby beauby merged commit b4e2ac3 into rails-api:master Apr 21, 2016
@NullVoxPopuli
Copy link
Contributor

👍

@beauby beauby deleted the meta-madness branch April 22, 2016 09:05
bf4 added a commit to bf4/active_model_serializers that referenced this pull request May 2, 2016
bf4 added a commit to bf4/active_model_serializers that referenced this pull request May 17, 2016
bf4 added a commit to bf4/active_model_serializers that referenced this pull request May 17, 2016
NullVoxPopuli pushed a commit that referenced this pull request May 17, 2016
* Assert Schema

* Fix regression from #1695 where JSONAPI renders empty meta

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

Successfully merging this pull request may close these issues.

3 participants