-
Notifications
You must be signed in to change notification settings - Fork 14
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
Drop MultiJson dependency #24
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Can you add a CHANGELOG entry, please? 😸 |
This should be good to merge once the CI is green. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specs will be failing so you might need to do more gymnastics as Grape::JSON
was not introduced until 1.0 version of Grape.
CHANGELOG.md
Outdated
@@ -1,7 +1,7 @@ | |||
Next | |||
---- | |||
|
|||
* Your contribution here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put this back.
Change the format of below to match every other line, please.
https://github.com/ruby-grape/grape-active_model_serializers/blob/master/lib/grape-active_model_serializers/error_formatter.rb#L19 might be helpful, although I am not in love with the solution, maybe define |
So why do we need to have a check against |
@mach-kernel it doesn't look like representable depends on the |
Representable actually requires |
It is no longer encouraged to use the
multi_json
gem due to this reason: intridea/multi_json#113 (comment)cc @dblock