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

add 404 existence guards for make-run server #576

Closed
wants to merge 1 commit into from

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented Jun 9, 2016

No description provided.

@zoffixznet
Copy link
Contributor

Does this PR bring any other benefits other than saving one redirect on 404s?

@zoffixznet
Copy link
Contributor

I asked on IRC, and it seems this app is not in use on the live site, but simply is a convenience feature for local use.

Since Mojolicious's static handler will already handle the 404's for non-existent files, there's no benefit for adding explicit check in this case.

I'll close this PR. If there are some benefits I missed, please reopen. Thanks.

@zoffixznet zoffixznet closed this Jun 9, 2016
@jsoref
Copy link
Contributor Author

jsoref commented Jun 10, 2016

I'm not sure how to reopen.

When using a link checker, a request for http://localhost:.../robots.txt gets sent to /robots.txt.html which gets sense to /robots.txt.html.html, etc. Thankfully my link checker gives up after 10 attempts, but it's pretty annoying.

@coke
Copy link
Collaborator

coke commented Jun 10, 2016

If the error doesn't happen on the production site it's probably not worth fixing

@jsoref
Copy link
Contributor Author

jsoref commented Jun 10, 2016

I tried visiting http://localhost:3000/language/ and ended up at: http://localhost:3000/language/.html.html.html.html.html.html.html.html.html.html.html.html.html.html.html.html.html.html.html.html.html

It's making trying to use the thing not worth my time. Hopefully you can see that I can make valuable contributions. But if the tools balk me, I'll leave.

zoffixznet added a commit that referenced this pull request Jun 10, 2016
@zoffixznet
Copy link
Contributor

zoffixznet commented Jun 10, 2016

Thanks for pointing out that issue. It's fixed in 9b72be0 fef2e8e. Try it out. If you're still experiencing problems, please open an Issue.

P.S.:

I'm not sure how to reopen.

You just click this button:
png

@jsoref
Copy link
Contributor Author

jsoref commented Jun 10, 2016

image

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