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

Docs: Fixes warnings and other noisy build messages #9453

Conversation

benjaoming
Copy link
Contributor

@benjaoming benjaoming commented Jul 26, 2022

  • Fixes warnings that broke builds
  • Fixes noisy sphinx-hoverxref message Using default style (tooltip) for unknown typ (term). Define it in hoverxref_role_types.
  • Updates intersphinx references to use "stable" or "latest" versions, rather than referencing outdated versions.

📚 Documentation preview 📚: https://docs--9453.org.readthedocs.build/en/9453/

@benjaoming benjaoming requested a review from a team as a code owner July 26, 2022 09:18
@benjaoming benjaoming requested a review from ericholscher July 26, 2022 09:18
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a useful PR, but I do think we probably want to point at versions here, instead of stable to avoid more breakage in the future.

docs/conf.py Show resolved Hide resolved
@benjaoming
Copy link
Contributor Author

This is a useful PR, but I do think we probably want to point at versions here, instead of stable to avoid more breakage in the future.

After seeing that we were linking to several outdated docs, I would think that it's better to risk the little breakage that this can cause once in a while. For instance, we are now linking to the latest Django release (4.0) and not Django 2.2. Updating these mappings didn't require many changes, and we won't be notified in any other way when we are linking to old versions. I think we should avoid the former scenario and find a good way for our docs to break when they contain a broken reference to another documentation's latest stable release.

I'd argue that it's the best practice, albeit not entirely reproducible?

Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a reasonable practice, but will lead to broken docs builds more frequently the more we link out. That leads to our public docs being broken until we fix the link and redeploy, which for stable is only once a week. Hopefully the downstream users will redirect those links though, but intersphinx doesn't have a way to handle redirects, so we won't benefit from it in our PR builds.

I'm fine with this for now, but if we see a lot of breakage, we should roll it back.

@benjaoming
Copy link
Contributor Author

Merging with the approval, I believe that we'll automatically see more to the issue discussed since references to (labels) the "latest" or "stable" branches of intersphinx-mapped documentation will break... we just don't know how often, but we will, and then we can potentially switch back to referencing a fixed version.

We'll also be able to discuss the reproducible aspect more -- note that we were never fully reproducible wrt Intersphinx, as many of those mappings were already "latest".

@benjaoming benjaoming merged commit 7e0fa4e into readthedocs:main Jul 29, 2022
@benjaoming benjaoming deleted the docs/fix-warnings-and-other-build-messages branch July 29, 2022 18:06
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.

2 participants