docs: add warning when rendering preview docs #2559
Conversation
docs/source/conf.py
Outdated
| is_dirty = bool(subprocess.check_output(["git", "status", "--untracked-files=no", "--porcelain"], text=True).strip()) | ||
| commit_hash = subprocess.check_output(["git", "rev-parse", "HEAD"], text=True).strip() | ||
| git_describe = subprocess.check_output(["git", "describe", "--tags"], text=True).strip() + ("-dirty" if is_dirty else "") |
There was a problem hiding this comment.
I have been very much enjoying the versioningit project over the last year or so, which can automate away this sort of stuff (at the expense of reading the version from a _version.py file or somesuch instead of using importlib metadata). But I tried to write this patch using only what's already present, and it should work fine as long as git is present and the docs are being built from the repository, and degrade gracefully if there are problems (i.e. you'll still get a warning box if git is not present or the CWD violates this code's assumptions)
The output of `git-describe` is in terms of the latest tag on the current branch, which can produce surprising results when tags are distributed across release branches (i.e. `v7.1.0-992-g51300a1a` is the best it can produce for that commit, even though the latest stable tag at that point is `v7.1.9`). It makes more sense to refer only to the commit.
Is that a generalized aesthetic concern, or are you thinking of specific failure modes here? An alternative that occurred to me while putting this PR together would be to add a Edit: per discussion on IRC, I will rewrite to shove as much |
SnoopJ
force-pushed
the
doc/preview-docs-warning-header
branch
from
b86c872
to
4009450
Compare
| if Version(__version__).is_prerelease or commit_hash: | ||
| tags.add("preview") |
There was a problem hiding this comment.
Note: with the change to an environment variable, the preview tag is added if the user set the environment variable, even if the __version__ is not a pre-release. It seems prudent that the output of make docs_preview would always set the tag
There was a problem hiding this comment.
Hm... this might sound stupid but: why do we need to check the commit hash when we can just check for the version number? It feels like an awful lot of works for something that shouldn't happen outside of the commit that bumps Sopel's version to a stable one.
So actually, I'm 👎 on the whole commit hash check.
| commit_hash = os.environ.get("SOPEL_GIT_COMMIT", "")[:7] | ||
| is_dirty = bool(os.environ.get("SOPEL_GIT_DIRTY")) | ||
| if commit_hash: | ||
| github_ref = "https://github.com/sopel-irc/sopel/commit/{commit_hash}".format(commit_hash=commit_hash) |
There was a problem hiding this comment.
It occurred to me while preparing my revisions that this GitHub link isn't guaranteed to be valid when building the preview docs against commits that aren't pushed to the repo, but I'm not sure that's worth worrying about, it should be valid in general for work we do in this repo (and the docs that end up on the website), and anybody hacking on a local clone of the source probably will understand if the link is invalid.
Description
I had not realized that the unstable documentation was being regularly built against
master, and I thought it would be helpful if these preview docs included a warning about their preview-ness as well as a reference to the specific commit they were built against.The warning added by this changeset is included on all pages using Sphinx's
rst_prologvariable, but only rendered when the Sopel version is a pre-release (i.e..devN) version.Below is an example of how preview documentation renders (the links points to the relevant commit on GitHub and https://sopel.chat/docs/, respectively)
And here's what the docs look like for a release version:
Checklist
make qa(runsmake lintandmake test)