-
Notifications
You must be signed in to change notification settings - Fork 476
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
Conversation
many thanks for your work @wojciechka … please fix rubocop offences and I'm happyo to mörge it :) |
@wojciechka … please wait, fixing of rubocop stuff is out of scope of this PR |
@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. |
thanks again @wojciechka … that's what I meant with out of scope ;) |
@LeFnord sure, just pushed an update ; also, I am curious, wouldn't 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. |
@LeFnord With the rubocop version fix it has passed all the tests now. Anything pending on my side? |
@wojciechka yhanks again :) … happy to mörge it |
* 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
* 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
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.