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.

Sign up for GitHub

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

Jump to bottom

add the the failing spec for the issue which related to API with :path versioning #216

Closed
wants to merge 3 commits into from
Closed

add the the failing spec for the issue which related to API with :path versioning #216

wants to merge 3 commits into from

Conversation

dm1try
Copy link
Member

@dm1try dm1try commented Feb 13, 2015

so when you specify api "format" and use "path" versioning then "root" endpoints disappear from swagger docs
related to grape >= 0.10.0

Run spec:

GRAPE_VERSION="~>0.10" bundle update grape
GRAPE_VERSION="~>0.10" bundle exec rspec spec/api_with_path_versioning_spec.rb

@dm1try
add the the failing spec for the issue which related to API with :pat…
717c62e 
…h versioning

so when you specify api "format" and use "path" versioning then "root" endpoints disapper from swagger docs
related to grape >= 0.10.0
@dm1try dm1try mentioned this pull request Feb 13, 2015
Closed
@dm1try
Copy link
Member Author

dm1try commented Feb 13, 2015

I checked this issue, seems it is related to grape-swagger.
grape < 0.10.0 add "(.:format)" postfix to the route path, nvm if you use only specific format.
after refactoring grape => 0.10.0 does not add this postfix, see - https://github.com/intridea/grape/blob/master/lib/grape/path.rb#L41

grape-swagger route matching with grape 0.9.0:

"route_path: /:version/resources(.:format)"
"route_match: /:version/resources(.:format)"
#<MatchData "/resources(" 1:"resources">

route matching with grape 0.10.0:

"route_path: /:version/resources"
"route_match: /:version/resources"
#<MatchData "/" 1:"">

so the regex fix from https://github.com/tim-vandecasteele/grape-swagger/issues/192 seems legit for me, I am going to apply it :)

Adam Weller and others added 2 commits February 13, 2015 15:58
@dm1try
Match index routes that do not contain :format, as it has been explic…
ea58020 
…itly

declared in the resource, e.g., `format :json`.

TODO:  Add spec(s).
@dm1try
update CHANGELOG for #216
10c0b94 
[ci skip]
@dm1try
Copy link
Member Author

dm1try commented Feb 13, 2015

@dblock take a look ☺️

dblock
dblock reviewed Feb 13, 2015
View reviewed changes
CHANGELOG.md
@@ -6,6 +7,11 @@
* [#196](https://github.com/tim-vandecasteele/grape-swagger/pull/196): If `:type` is omitted, see if it's available in `:using` - [@jhollinger](https://github.com/jhollinger).
* [#200](https://github.com/tim-vandecasteele/grape-swagger/pull/200): Treat `type: Symbol` as string form parameter - [@ypresto](https://github.com/ypresto).
* [#207](https://github.com/tim-vandecasteele/grape-swagger/pull/207): Support grape `mutually_exclusive` - [@mintuhouse](https://github.com/mintuhouse).

#### Fixes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're going to do this, move fixes from above down here.

@dblock
Copy link
Member

dblock commented Feb 13, 2015

This is good, amend CHANGELOG as above and it's good to merge. Thanks for writing the tests.

dblock pushed a commit that referenced this pull request Feb 13, 2015
@dm1try @dblock
update CHANGELOG for #216
c9fb720 
[ci skip]
@dblock
Copy link
Member

dblock commented Feb 13, 2015

I realize I am being particularly annoying with this CHANGELOG thing, so I did it myself, tim-vandecasteele@c9fb720. Thanks @minch for the fix and @dm1try for the specs!

@dblock dblock closed this Feb 13, 2015
@dblock dblock mentioned this pull request Feb 13, 2015
Closed
@dm1try
Copy link
Member Author

dm1try commented Feb 13, 2015

so I did it myself, #c9fb720.

@dblock looks good) Thx!

@dm1try dm1try deleted the add_failing_spec_for_api_with_path_versioning branch February 13, 2015 22:46
@minch
Copy link
Contributor

minch commented Feb 13, 2015

👍

@dm1try dm1try mentioned this pull request Feb 15, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dm1try @dblock @minch