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

Require Grape 0.16.1 and fix route_xxx deprecations. #368

Closed
wants to merge 4 commits into from

Conversation

dblock
Copy link
Member

@dblock dblock commented Apr 4, 2016

This isn't ready to merge. there're issues with route_params and route_hidden.

Closes #364.

@dblock
Copy link
Member Author

dblock commented Apr 5, 2016

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
Copy link
Member

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

Copy link
Member Author

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

Copy link
Member

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

Copy link
Member Author

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.

@dblock dblock force-pushed the fix-deprecations branch 2 times, most recently from c21bea5 to 332d007 Compare April 9, 2016 23:23
@dblock
Copy link
Member Author

dblock commented Apr 9, 2016

@LeFnord once this is all fixed here, should I be merging it onto master or swagger-1.2?

@LeFnord
Copy link
Member

LeFnord commented Apr 10, 2016

@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

@dblock
Copy link
Member Author

dblock commented Apr 11, 2016

Closing in favor of #375.

@dblock dblock closed this Apr 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants