-
Notifications
You must be signed in to change notification settings - Fork 60.3k
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 managing-a-custom-domain-for-your-github-pages-site.md #34767
Conversation
Thanks for opening this pull request! A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines. |
Automatically generated comment ℹ️This comment is automatically generated and will be overwritten every time changes are committed to this branch. The table contains an overview of files in the Content directory changesYou may find it useful to copy this table into the pull request summary. There you can edit it to share links to important articles or changes and to give a high-level overview of how the changes in your pull request support the overall goals of the pull request.
fpt: Free, Pro, Team |
...tom-domain-for-your-github-pages-site/managing-a-custom-domain-for-your-github-pages-site.md
Show resolved
Hide resolved
@BenjaminBrienen Thank you very much for the fix! ✨ This repo is currently in a code freeze, but we will get this merged as soon as the freeze ends on 10/7! |
| Subdomain<br />(`www.example.com`,<br />`blog.example.com`) | `CNAME` | `SUBDOMAIN` | `USERNAME.github.io` or<br /> `ORGANIZATION.github.io` | | ||
| Subdomain<br />(`www.example.com`,<br />`blog.example.com`) | `CNAME` | `SUBDOMAIN.example.com.` | `USERNAME.github.io` or<br /> `ORGANIZATION.github.io` | |
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.
FYI this was recently removed in #31358 — so just a heads up for triage to hopefully settle on a style and avoid changing it back and forth…
(TBH: I'd personally like to see it added back like in this PR, though…)
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.
@janbrasna Thank you for the reminder @janbrasna! 💛 Let me put this up for review so we can get a second opinion on this
CC @BenjaminBrienen just for visibility (thanks for your patience!)
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'm sure a little extra review on this isn't blocking anyone 😁
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.
For the record… one half of the above mentioned PR got reverted pretty soon (something I didn't like back then, too) so this would revert the remaining change — and I see it as a good thing — the consistency callout with that specific page/table is my exact point from few months ago, too;)
And as DNS management UIs differ between providers there's not exactly one "correct" format for it per se to be completely copy-paste-ready all the time, so it's not worth fiddling with it too much beyond some basic correctness and consistency across the pages (in the future).
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 for bearing with us (me) @janbrasna 💛 we'll hopefully have a concrete direction on this soon to be abide by for the foreseeable future (outside of basic correctness and consistency like you mentioned)
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 so much @janbrasna and apologies for the confusion here—I'm not honestly sure what's happened but good that we're sorting it now.
And as DNS management UIs differ between providers there's not exactly one "correct" format for it per se to be completely copy-paste-ready all the time, so it's not worth fiddling with it too much beyond some basic correctness and consistency across the pages (in the future).
Agree with you completely here, so I'm happy to go ahead and make this change after the freeze Alex mentioned. Thanks for picking this up, @BenjaminBrienen—we'll merge this next week 👍
Thanks very much for contributing! Your pull request has been merged 🎉 You should see your changes appear on the site in approximately 24 hours. If you're looking for your next contribution, check out our help wanted issues ⚡ |
Why:
Consistency with https://docs.github.com/en/pages/getting-started-with-github-pages/securing-your-github-pages-site-with-https
What's being changed (if available, include any code snippets, screenshots, or gifs):
Check off the following:
I have reviewed my changes in staging, available via the View deployment link in this PR's timeline (this link will be available after opening the PR).
data
directory.For content changes, I have completed the self-review checklist.