-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Remove logic for guessing slug from an unregistered domain #5143
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
Remove logic for guessing slug from an unregistered domain #5143
Conversation
We can remove the |
I think this is ready to go, but I think we need to do a bit more work to reduce the possible downsides. We have some manual migration steps, but any project that is missed will randomly have their Domain stop working. This is not a great experience, so I think we should:
|
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.
Solid changes. 👍
please review our <a href="{{ docsurl }}">custom domain documentation</a>. | ||
In the past, we allowed custom domains to point to us without configuring the domain in the Read the Docs dashboard | ||
and we attempted to intelligently guess the correct project. | ||
This was never explicitly supported and documented. It just happened to work. |
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.
Believe it used to be documented.
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 can update that then
We have logs over the last 3 months. I added the domain where the following were true:
|
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 added to this PR but I'll selfishly approve it. We should probably wait on at least one more approval.
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.
Believe I am 👍 on this change. We should watch for DNS errors after the deploy though, and make sure we don't break anyone important.
Going to fix the tests |
<p> | ||
{% with docsurl='https://docs.readthedocs.io/en/stable/custom_domains.html' %} | ||
{% blocktrans trimmed %} | ||
If you control this domain and believe this is in error, |
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.
Shouldn't be is an error
?
If you control this domain and believe this is in error, | |
If you control this domain and believe this is an error, |
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.
Either works but I think yours is better.
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.
Interesting, I guess is used more in other areas?
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 messed up #4730 trying to solve the conflicts. See the another PR for the discussion (sorry!)
Close #1991