-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Take down docker.github.io #6433
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
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.
thanks! I'm not super-familiar with the Jekyll options, but left some comments on the HTML changes 👍
index.html
Outdated
<meta http-equiv="refresh" content="0; url=https://docs.docker.com" /> | ||
<title>Docker docs</title> | ||
</head> | ||
<body></body> |
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.
Perhaps this body should also mention that we're redirecting; similar to the 404 page, and provide a link if redirect doesn't kick in (e.g., if you're not being automatically redirected, click here ..)
404.html
Outdated
<html lang="en"> | ||
<head> | ||
<meta charset="UTF-8"> | ||
<title>Docker docs</title> |
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.
nit:perhaps Docker Documentation
instead of "docs"
index.html
Outdated
<head> | ||
<meta charset="UTF-8"> | ||
<meta http-equiv="refresh" content="0; url=https://docs.docker.com" /> | ||
<title>Docker docs</title> |
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.
nit:perhaps Docker Documentation
instead of "docs"
<!DOCTYPE html> | ||
<html lang="en"> | ||
<head> | ||
<meta charset="UTF-8"> |
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.
Should we have a meta-redirect here as well (perhaps with a 1..2 seconds timeout)?;
<meta http-equiv="refresh" content="1; url=https://docs.docker.com" />
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.
I put it for 1 second as you suggested; should be enough time for JS to kick in if enabled.
404.html
Outdated
<p>We have moved away from the <strong>docker.github.io</strong> domain. Please visit <a href="https://docs.docker.com">docs.docker.com</a> instead.</p> | ||
|
||
<script> | ||
window.location.replace("https://docs.docker.com" + window.location.pathname); |
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.
Not sure we want to remove the original URL from history; perhaps use .assign()
instead?
window.location.assign("https://docs.docker.com" + window.location.pathname);
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.
LGTM, but would like people from the docs team to verify the Jekyll side of things 😅
@JustinINevill also sounds like a good candidate for discussion during F2F. See #6101 for more context. |
@allejo I looked into doing a similar redirect for one of the DTR pages but according to http://www.redirect-checker.org/can-i-use-meta-refresh.php and Wikipedia, the meta refresh tag is deprecated and not recommended by W3C and Google. I do find it concerning that |
Yea, the correct way of handling redirects is using response headers instead of the meta tag or JS. However, GitHub pages doesn’t support anything fancy like response headers. Their recommendation for redirects is the Jekyll redirect plugin, which at its core uses a meta tag refresh. Unless GitHub support gets involved and starts supporting response redirect support, not sure what other options there are. |
Deploy preview for docsdocker ready! Built with commit fef6260 |
Yes, it's definitely a dirty-hack® / band-aid, but I don't think there's another way currently to remove the docker.github.io domain (see the discussion on #6101) |
@bermudezmt this LGTY? |
@allejo @thaJeztah what's the least invasive way to test this change? I fiddled around with testing this locally but that doesn't really work. I don't suppose we could point the site URL in |
One idea would be replicating the situation this repo is in:
|
@allejo thank you for the quick response! Sorry if you're having to repeat information regarding this issue (and PR). I will bring this up again at our next team meeting and let you know how we decide to move forward. |
Status update 01/14/2019:
Next step is for @JustinINevill to check on current site traffic for |
Just as a note, the |
@allejo thank you very much for your work on this! @bermudezmt I'm approving the implementation of this based on your testing and the reduced user confusion that will result from removing this now superfluous, not fully working, and low traffic domain. |
Thank you so much for your patience with this, @allejo . We welcome more web development / Jekyll insights from you! |
Proposed changes
Don't know if you're still interested in taking down the
docker.github.io
domain from #6101, but see my comment #6101 (comment).This PR does the following:
.nojekyll
so GitHub doesn't build the site as a Jekyll siteindex.html
so GitHub has something to fall back on and it'll redirect todocs.docker.com
index.html
to Jekyll's exclude list so it doesn't interfere withindex.md
404.html
to blindly attempt a redirect to a docs.docker.com URLCloses #6101
/cc @joaofnfernandes @thaJeztah