-
Notifications
You must be signed in to change notification settings - Fork 471
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
Require Grape 0.16.1 and fix route_xxx deprecations. #368
Conversation
For now I am monkey-patching the deprecation to avoid too many warnings being spewed. module Grape
class Router
class Route
private
# TODO: remove when https://github.com/ruby-grape/grape-swagger/pull/368 and https://github.com/newrelic/rpm/pull/235 are finished
def warn_route_methods(name, location, expected = nil)
end
end
end
end |
if route.route_entity | ||
type = @@documentation_class.parse_entity_name(Array(route.route_entity).first) | ||
if route.route_is_array | ||
if route.entity |
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.
please respect also route.success
→ see: describing-methods
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.
that needs a spec I suspect, wasn't caught
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.
success
is the new entity
and failures
the new http_codes
, think a small spec to proof both new work in the same way as the old one should be enough
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.
Actually no need, these were declared as DSL sugar on top of the entity
and http_codes
implementation in https://github.com/ruby-grape/grape/blob/1097eb192a8601234b5465c23b1ab09101048792/lib/grape/dsl/desc.rb#L90, so they work today out of the box.
c21bea5
to
332d007
Compare
@LeFnord once this is all fixed here, should I be merging it onto master or swagger-1.2? |
c13cf86
to
ae920ab
Compare
@dblock please merge to swagger-1.2, so we could release the actual master as 0.10.5, and this work for 0.11.0 |
ae920ab
to
07f0356
Compare
07f0356
to
d144452
Compare
Closing in favor of #375. |
This isn't ready to merge. there're issues with
route_params
androute_hidden
.Closes #364.