Skip to content

Fix mounting APIs in route_param namespaces #634

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

Merged
merged 7 commits into from
Sep 26, 2017
Merged

Fix mounting APIs in route_param namespaces #634

merged 7 commits into from
Sep 26, 2017

Conversation

wojciechka
Copy link
Contributor

This is based on #543 by @milgner and addresses issue #267

I have applied the changes on top of latest master and included test cases.

@coveralls
Copy link

Coverage Status

Coverage increased (+2.0%) to 99.642% when pulling b29a8ed on wojciechka:fix267 into e9d9447 on ruby-grape:master.

@ruby-grape ruby-grape deleted a comment from coveralls Sep 25, 2017
@ruby-grape ruby-grape deleted a comment from coveralls Sep 25, 2017
@ruby-grape ruby-grape deleted a comment from coveralls Sep 25, 2017
@ruby-grape ruby-grape deleted a comment from coveralls Sep 25, 2017
@ruby-grape ruby-grape deleted a comment from coveralls Sep 25, 2017
@ruby-grape ruby-grape deleted a comment from coveralls Sep 25, 2017
@LeFnord
Copy link
Member

LeFnord commented Sep 25, 2017

many thanks for your work @wojciechka … please fix rubocop offences and I'm happyo to mörge it :)

@LeFnord
Copy link
Member

LeFnord commented Sep 25, 2017

@wojciechka … please wait, fixing of rubocop stuff is out of scope of this PR

@wojciechka
Copy link
Contributor Author

@LeFnord I have just pushed fixes that clean up offences related to my changes.

As you have probably noticed, most of the issues are not related to the changes in code I have made.

But I also agree that I should not be introducing new offences.

@ruby-grape ruby-grape deleted a comment from coveralls Sep 26, 2017
@ruby-grape ruby-grape deleted a comment from coveralls Sep 26, 2017
@ruby-grape ruby-grape deleted a comment from coveralls Sep 26, 2017
@ruby-grape ruby-grape deleted a comment from coveralls Sep 26, 2017
@ruby-grape ruby-grape deleted a comment from coveralls Sep 26, 2017
@LeFnord
Copy link
Member

LeFnord commented Sep 26, 2017

thanks again @wojciechka … that's what I meant with out of scope ;)
please can you fix the rubocop version to 0.49.1, so all should be fine

@wojciechka
Copy link
Contributor Author

@LeFnord sure, just pushed an update ; also, I am curious, wouldn't Gemfile.lock prevent those things from happening (as in an update in rubocop version triggering a lot of false positives)?

I usually commit Gemfile.lock to avoid package versions changing between deployments etc and I saw it explicitly removed.

It's not really related to this PR, but I am curious about it.

@coveralls
Copy link

Coverage Status

Coverage increased (+2.0%) to 99.642% when pulling c5747cd on wojciechka:fix267 into e9d9447 on ruby-grape:master.

@wojciechka
Copy link
Contributor Author

@LeFnord With the rubocop version fix it has passed all the tests now.

Anything pending on my side?

@ruby-grape ruby-grape deleted a comment from coveralls Sep 26, 2017
@ruby-grape ruby-grape deleted a comment from coveralls Sep 26, 2017
@ruby-grape ruby-grape deleted a comment from coveralls Sep 26, 2017
@ruby-grape ruby-grape deleted a comment from coveralls Sep 26, 2017
@LeFnord
Copy link
Member

LeFnord commented Sep 26, 2017

@wojciechka yhanks again :) … happy to mörge it

@LeFnord LeFnord merged commit 7d871ae into ruby-grape:master Sep 26, 2017
@rypit
Copy link

rypit commented Oct 28, 2017

Also addresses issues similar to #610 & #508 which were happening for me in 0.27.3. Thanks for this 👍

hara-y-u pushed a commit to hara-y-u/grape-swagger that referenced this pull request Dec 8, 2017
* Fix mounting APIs in route_param namespaces

* Added missing tests, based on code from

ruby-grape#543 by @milgner

Applied on top of latest master

* Added changelog

* Fix CHANGELOG.md

* Cleanup rubocop offences related to this change

* Fix rubocop version

* Re-added * Your contribution here to CHANGELOG.md
LeFnord pushed a commit to LeFnord/grape-swagger that referenced this pull request Feb 9, 2019
* Fix mounting APIs in route_param namespaces

* Added missing tests, based on code from

ruby-grape#543 by @milgner

Applied on top of latest master

* Added changelog

* Fix CHANGELOG.md

* Cleanup rubocop offences related to this change

* Fix rubocop version

* Re-added * Your contribution here to CHANGELOG.md
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.

5 participants