-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add version warning banner for "latest" sphinx docs #1346
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
"<p class='last'> " + | ||
"This document is for an <strong>unreleased development version</strong>. " + | ||
"Documentation is available for the <a href='/en/stable/'>current stable release</a>, " + | ||
"or for older versions through the “v:” menu at bottom left." + |
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 text leaves the reader to infer that documentation for an "unreleased development version" may not be appropriate for their use case. Should we explicitly state this? Open to suggestions.
"Documentation is available for the <a href='/en/stable/'>current stable release</a>, " + | ||
"or for older versions through the “v:” menu at bottom left." + | ||
"</p>"; | ||
warning.querySelector('a').href = window.location.pathname.replace('/latest', '/stable'); |
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 line is a bit heavy-handed in that it replaces any and all instances (not just the relevant instance) of /latest
with /stable
in the url. So if we someday have a page that happens to contain an unrelated "/latest" (e.g. /en/latest/auto_examples/latest_CEC_modules.html
or something), it would blindly edit both instances. I guess an easy improvement is to replace /en/latest
with /en/stable
. I guess the most robust fix would include some semantic understanding of URL structure. Maybe this is much ado about something that will probably never happen.
Looks good to me. Alternatively, readthedocs will let us hide the latest version. Does anyone use it other than developers? |
Apparently they do; a quick look through a data dump of RTD's traffic analytics reveals that traffic to "latest" is ~1/10th that of "stable", and more than any numeric version. Numbers here are total page views since 2021-09-05.
It's tangential to this PR, but anyone have an explanation for the nontrivial amount of traffic going to versions as old as 0.1 and 0.2.x? |
v0.7 dropped support for python 2.7, and in v0.6 we deprecated a number of functions for renaming or replacement. Maybe the traffic on v0.6.0 docs is for users who have decided not to update their code. The traffic for 0.2 is perplexing. |
Needed a couple updates for #1173. I'll merge this in a couple days unless anyone objects. This is what it looks like now: |
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.
Do we need to include the BSD3 license of either source?
Done, thanks for the reminder! |
Tests addedUpdates entries todocs/sphinx/source/api.rst
for API changes.Adds description and name entries in the appropriate "what's new" file indocs/sphinx/source/whatsnew
for all changes. Includes link to the GitHub Issue with:issue:`num`
or this Pull Request with:pull:`num`
. Includes contributor name and/or GitHub username (link with:ghuser:`user`
).New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.This PR adds a warning banner on
latest
docs pages using the method linked in #1260, i.e. checking the RTD metadata on page load and dynamically generating the banner if RTD indicates the page is in thelatest
version. That means that the banner won't show up in older docs versions or PR builds (both of which already get appropriate warning banners from RTD) and local builds (where it doesn't really matter anyway).Because it won't show up on the PR build, I did a test build locally with the version check commented out (so that the banner is always generated) and here is the result: