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

Fix: 652 - Incorrect error on route with no versions #1465

Merged
merged 4 commits into from Sep 13, 2017

Conversation

karthikiyengar
Copy link
Contributor

Pre-Submission Checklist

  • Opened an issue discussing these changes before opening the PR
  • Ran the linter and tests via make prepush
  • Included comprehensive and convincing tests for changes

Issues

Closes: #652

Changes

Throws in InvalidVersionError instead of a MethodNotAllowedError when a route with no versions is called with the 'Accept-Version' header

@DonutEspresso
Copy link
Member

Thank you for taking this up, @karthikiyengar! Always great to see new contributors. 🎉

What's your comfort level on adding tests that cover this new behavior? It would be great to see a test for both a mismatching version as well as a missing version, as reported originally in #652.

@karthikiyengar
Copy link
Contributor Author

@DonutEspresso - Sure, I can add some tests. Thank you for the library, I've had much fun using it :-)

@karthikiyengar
Copy link
Contributor Author

Tests added 😄

@retrohacker
Copy link
Member

🎉 This is awesome @karthikiyengar !!!!!!

Thanks for doing this, the PR looks good to me!

Copy link
Member

@retrohacker retrohacker left a comment

Choose a reason for hiding this comment

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

🎉 :shipit:

@DonutEspresso
Copy link
Member

Fantastic. Thanks @karthikiyengar! 🎉

@DonutEspresso DonutEspresso merged commit ee15490 into restify:master Sep 13, 2017
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.

None yet

3 participants