-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Update README to fix 404 error issue mounting Grape Using Rack::Cascade #2089
Conversation
1762e86
to
711f392
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I have some nitpicks if you could please fix.
CHANGELOG.md
Outdated
@@ -7,6 +7,8 @@ | |||
#### Fixes | |||
|
|||
* Your contribution here. | |||
* [#2089](https://github.com/ruby-grape/grape/pull/2089): Update README to fix 404 error issue mounting grape using rack::cascade - [@jonmchan](https://github.com/jonmchan). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rack::Cascade is capitals and quote it as Rack::Cascade
Error and issue is redundant ;) Maybe "Specify order of mounting Grape with Rack::Cascade
in README"?
README.md
Outdated
``` | ||
|
||
**NOTE:** Order of loading apps using `Rack::Cascade` matters. The grape application must be last if you want to raise custom 404 errors from grape (such as `error!('Not Found',404)`). If the grape application is not last and returns 404 or 405 response, [cascade utilizes that as a signal to try the next app](https://www.rubydoc.info/gems/rack/Rack/Cascade). This may lead to undesirable behavior showing the [wrong 404 page from the wrong app](https://github.com/ruby-grape/grape/issues/1515). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't use NOTE
like this elsewhere, just say Note that the order of loading ...
.
Capitalize Grape.
711f392
to
17c7190
Compare
…-1515-update_doc
Fixed |
Closes #1515.