Allow symbols for status codes #950
Conversation
dabrorius
force-pushed
the
allow_symbols_for_status_codes
branch
from
9b3bfb5
to
19a43fb
Compare
|
I like it. Rebase and squash please? |
dabrorius
force-pushed
the
allow_symbols_for_status_codes
branch
2 times, most recently
from
a044ac8
to
d23f5b6
Compare
|
Done |
| @@ -5,6 +5,7 @@ | |||
|
|
|||
| * [#936](https://github.com/intridea/grape/pull/936): Fixed default params processing for optional groups - [@dm1try](https://github.com/dm1try). | |||
| * [#942](https://github.com/intridea/grape/pull/942): Fixed forced presence for optional params when based on a reused entity that was also required in another context - [@croeck](https://github.com/croeck). | |||
| * [#950](https://github.com/intridea/grape/pull/950): Status method can now accept one of Rack::Utils status code symbols (:ok, :found, :bad_request, etc.) - [@dabrorius](https://github.com/dabrorius) | |||
There was a problem hiding this comment.
OCD: missing period after the line.
dabrorius
force-pushed
the
allow_symbols_for_status_codes
branch
from
d23f5b6
to
e83cd2f
Compare
| if Rack::Utils::SYMBOL_TO_STATUS_CODE.keys.include?(status) | ||
| @status = Rack::Utils.status_code(status) | ||
| else | ||
| fail ArgumentError, "Status code :#{status} is not valid." |
There was a problem hiding this comment.
You want to specialize this error into Grape::Exceptions and make it localizable too, https://github.com/intridea/grape/blob/master/lib/grape/locale/en.yml.
I think not valid should be invalid, too.
There was a problem hiding this comment.
This is an error that only developer should see, not the end user. Is there really need to localize it?
I used this as reference https://github.com/intridea/grape/blob/master/lib/grape/dsl/inside_route.rb#L28
dabrorius
force-pushed
the
allow_symbols_for_status_codes
branch
2 times, most recently
from
3876c88
to
926a92f
Compare
|
@dblock I changed the error message but didn't localize it yet. As I commented before, I'm not sure if it makes sense to allow user to translate a message that API end-users (as far as I understand) will never see. If it really needs to be localized I can change it but I wanted to make sure you really want that. |
dabrorius
force-pushed
the
allow_symbols_for_status_codes
branch
from
926a92f
to
244671c
Compare
|
Fair enough, merging. |
|
Good one @dabrorius 👍 |
|
Thanks :) |
This is a very simple implementation of feature requested in #934.