-
Notifications
You must be signed in to change notification settings - Fork 472
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
Default success codes to be excluded when http_codes define one already #641
Conversation
1f65109
to
35ca182
Compare
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.
Thanks @anakinj … but have one question, see comment
let(:app) do | ||
Class.new(Grape::API) do | ||
desc 'Has explicit http_codes defined' do | ||
http_codes [{ code: 202, message: 'We got it!' }, |
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.
what's happened if we have mixed codes, something like this:
http_codes [
{ code: 200, message: 'all fine' },
{ code: 404, message: 'Not found' }
]
maybe adding a spec to document the behaviour
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.
I've added some more status codes combinations to the specs
35ca182
to
a38c704
Compare
…es define one already
a38c704
to
1aab0ee
Compare
thanks @anakinj |
…dy (ruby-grape#641) * Do not include the default success codes to the responses if http_codes define one already * Added spec with success and error codes
…dy (ruby-grape#641) * Do not include the default success codes to the responses if http_codes define one already * Added spec with success and error codes
It was not possible to override a POSTs default 201 status code. This change will prevent the default success code to be added if the http_codes array already include a success code.