-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Use canonical URL from html_baseurl #1003
Conversation
94e40aa
to
54f7ead
Compare
54f7ead
to
5421db8
Compare
We should at least be alerting the user, how are they supposed to know that something has changed and their canonical URL is now broken? |
I think we are aiming 1.0 here soon? If not, I think we add a warning is |
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 -1 on this change without a deprecation path. We can't just remove features without warning users and shipping a version that works and has a warning.
d285196
to
5f17c4a
Compare
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.
This looks better 👍
This is in sphinx itself now https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-html_baseurl,
https://github.com/sphinx-doc/sphinx/blob/e886cc21710e23bc53af83d011131828b1020848/sphinx/themes/basic/layout.html#L135-L137
I was thinking in deprecate this at first, but the change is simple enough to migrate.