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

improve documentation #15

Closed
wants to merge 1 commit into from
Closed

Conversation

s1037989
Copy link
Contributor

This probably isn't the best message, but I think it could be really helpful to be specifically clear about this example. I don't think it's immediately clear that the webhook URL that you specify in GitHub should not be of the same host name as one of the mounted apps. When it is incorrectly configured this way, GitHub merely gets an unhelpful 404 response. Of course the reason why is that the mounted app picks up on the host name specified in the Github webhook and of course that route is not configured; but this isn't necessarily obvious, nor is the solution: pick an altogether separate hostname such as prepending _ to the hostname as in http://_my_app.example.com/reload/1/2/3/4/5 or perhaps some other preferred recommendation.

This probably isn't the best message, but I think it could be really helpful to be specifically clear about this example.  I don't think it's immediately clear that the webhook URL that you specify in GitHub should not be of the same host name as one of the mounted apps.  When it is incorrectly configured this way, GitHub merely gets an unhelpful 404 response.  Of course the reason why is that the mounted app picks up on the host name specified in the Github webhook and of course that route is not configured; but this isn't necessarily obvious, nor is the solution: pick an altogether separate hostname such as prepending _ to the hostname as in http://_my_app.example.com/reload/1/2/3/4/5 or perhaps some other preferred recommendation.
@jhthorsen jhthorsen self-assigned this May 25, 2015
@jhthorsen jhthorsen closed this in e4acc69 May 25, 2015
@jhthorsen
Copy link
Owner

Thanks! I hope my version is OK. If not: please comment 👍

jhthorsen pushed a commit that referenced this pull request May 25, 2015
 - Fix init script recipe in Toadfarm::Manual::RunningToadfarm
 - Improved Reload plugin documentation, regarding WebHook URL #15
 - Will not run t/reload.t without TEST_RELOAD=1 #13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants