-
Notifications
You must be signed in to change notification settings - Fork 114
Limit the multiversion docs build to the newest minor versions #869
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
Changes from all commits
9e3235e
cca5ad2
588085d
3e82d85
6feb71c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| <!-- | ||
| Copyright 2026 The PyAthena authors | ||
|
|
||
| Licensed under the MIT License. | ||
| See LICENSE or https://opensource.org/licenses/MIT. | ||
|
|
||
| SPDX-License-Identifier: MIT | ||
| --> | ||
| <!DOCTYPE html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="utf-8"> | ||
| <title>Page not found - PyAthena</title> | ||
| <script> | ||
| // A missing page under a version directory, such as a version that is | ||
| // no longer built, redirects to the same path on master. | ||
| (function () { | ||
| var match = window.location.pathname.match(/^\/v\d+\.\d+\.\d+(\/.*)?$/); | ||
| if (match) { | ||
| window.location.replace( | ||
| "/master" + (match[1] || "/") + window.location.search + window.location.hash | ||
| ); | ||
| } | ||
| })(); | ||
| </script> | ||
| </head> | ||
| <body> | ||
| <h1>Page not found</h1> | ||
| <p>See the <a href="/master/index.html">latest PyAthena documentation</a>.</p> | ||
| </body> | ||
| </html> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
| # | ||
| # For the full list of built-in configuration values, see the documentation: | ||
| # https://www.sphinx-doc.org/en/master/usage/configuration.html | ||
| import re | ||
| import subprocess | ||
| from datetime import datetime, timezone | ||
|
|
||
|
|
@@ -90,9 +91,48 @@ def config_inited(app, config): | |
| config.release = f"v{ver}" | ||
|
|
||
|
|
||
| def _parse_version_tag(name): | ||
| """Parse a release tag name. | ||
|
|
||
| Args: | ||
| name: Git ref name, e.g. ``v3.36.0`` or ``master``. | ||
|
|
||
| Returns: | ||
| The ``(major, minor, patch)`` integers, or None if the name is not a | ||
| ``vX.Y.Z`` tag. | ||
| """ | ||
| match = re.fullmatch(r"v(\d+)\.(\d+)\.(\d+)", name) | ||
| return tuple(int(part) for part in match.groups()) if match else None | ||
|
|
||
|
|
||
| def add_versions_newest_first(app, pagename, templatename, context, doctree): | ||
| """Handler for html-page-context event to order the version switcher. | ||
|
|
||
| Adds ``versions_newest_first`` to the template context: the branches | ||
| (``master``) first, then the tags from the newest version to the oldest. | ||
| sphinx-multiversion's own ``versions`` lists tags in ref name order, which | ||
| puts the oldest first and would sort ``v4.10.0`` before ``v4.9.0``. | ||
|
|
||
| Args: | ||
| app: Sphinx application. | ||
| pagename: Name of the page being rendered. | ||
| templatename: Name of the page template. | ||
| context: Template context, updated in place. | ||
| doctree: Doctree of the page, or None for generated pages. | ||
| """ | ||
| versions = context.get("versions") | ||
| if versions: | ||
| context["versions_newest_first"] = [ | ||
| *versions.branches, | ||
| *sorted(versions.tags, key=lambda item: _parse_version_tag(item.name), reverse=True), | ||
| ] | ||
|
|
||
|
|
||
| def setup(app): | ||
| """Sphinx setup hook.""" | ||
| app.connect("config-inited", config_inited) | ||
| # Run after sphinx-multiversion adds ``versions`` at the default priority | ||
| app.connect("html-page-context", add_versions_newest_first, priority=600) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Self-review round one, expanded scope (implementation behavior): CLEAN Scope: the full diff Checked:
Findings: none. |
||
|
|
||
|
|
||
| # -- Project information ----------------------------------------------------- | ||
|
|
@@ -214,8 +254,93 @@ def setup(app): | |
|
|
||
| # -- Sphinx-multiversion configuration ---------------------------------------- | ||
|
|
||
| # Whitelist pattern for tags (semantic versioning: vX.Y.Z) | ||
| smv_tag_whitelist = r"^v\d+\.\d+\.\d+$" # Match vX.Y.Z tags | ||
| # Number of minor versions whose latest patch release is documented | ||
| SMV_MINOR_VERSIONS = 3 | ||
|
|
||
|
|
||
| def _select_documented_tags(count): | ||
| """Select the version tags to document. | ||
|
|
||
| Picks the latest patch tag of each of the newest ``count`` minor versions, | ||
| e.g. ``v3.36.0``, ``v3.35.4`` and ``v3.34.0``. The latest tag of the | ||
| previous major version is added when those minor versions do not include | ||
| it and it has the Sphinx documentation, e.g. ``v3.36.1`` after ``v4.2.0``, | ||
| ``v4.1.0`` and ``v4.0.0``. | ||
|
|
||
| Args: | ||
| count: Number of minor versions to document. | ||
|
|
||
| Returns: | ||
| The selected tag names, newest first. Empty when git is unavailable | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Independent review (relayed): CLEAN; one non-actionable observation repaired
Covered: all four changed files; initial config loading, exported per-ref config, per-version subprocesses, and the switcher metadata; tag ordering, multiple majors, maintenance releases, non-matching tags, and an empty tag set; 404 placement, custom-domain paths, query/hash preservation, and loop prevention; the local build, docs-lint, deploy filtering, and the changed docs. Verdict CLEAN. Non-actionable observations:
Independent follow-up on the repair: same reviewer and settings, session |
||
| or the configuration directory is not in a git repository, as when | ||
| sphinx-multiversion reads each version's configuration from its | ||
| exported tree. Only the selection from the invoking checkout is used. | ||
| """ | ||
| try: | ||
| result = subprocess.run( | ||
| ["git", "tag", "--list", "v*"], | ||
| capture_output=True, | ||
| text=True, | ||
| check=True, | ||
| ) | ||
| except (subprocess.CalledProcessError, FileNotFoundError): | ||
| return [] | ||
|
|
||
| versions = sorted( | ||
| ( | ||
| (version, tag) | ||
| for tag in result.stdout.split() | ||
| if (version := _parse_version_tag(tag)) is not None | ||
| ), | ||
| reverse=True, | ||
| ) | ||
| if not versions: | ||
| return [] | ||
|
|
||
| # Newest first, so the first tag seen for each minor version is its latest patch | ||
| latest = {} | ||
| for (major, minor, _), tag in versions: | ||
| latest.setdefault((major, minor), tag) | ||
| selected = list(latest.values())[:count] | ||
|
|
||
| newest_major = versions[0][0][0] | ||
| previous_major_latest = next( | ||
| (tag for (major, _, _), tag in versions if major < newest_major), None | ||
| ) | ||
| if ( | ||
| previous_major_latest | ||
| and previous_major_latest not in selected | ||
| and _has_sphinx_docs(previous_major_latest) | ||
| ): | ||
| selected.append(previous_major_latest) | ||
| return selected | ||
|
|
||
|
|
||
| def _has_sphinx_docs(tag): | ||
| """Return whether a tag contains the Sphinx documentation. | ||
|
|
||
| Tags before ``v3.5.0``, including all ``v2`` tags, have no ``docs/conf.py``. | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Self-review round two, expanded scope (claims and operational behavior): CLEAN Claims checked against evidence at
Findings: none. |
||
|
|
||
| Args: | ||
| tag: Git tag name. | ||
|
|
||
| Returns: | ||
| True if ``docs/conf.py`` exists in the tag. | ||
| """ | ||
| result = subprocess.run( | ||
| ["git", "cat-file", "-e", f"{tag}:docs/conf.py"], | ||
| capture_output=True, | ||
| ) | ||
| return result.returncode == 0 | ||
|
|
||
|
|
||
| # Whitelist pattern for tags: only the tags selected above, or none | ||
| _documented_tags = _select_documented_tags(SMV_MINOR_VERSIONS) | ||
| smv_tag_whitelist = ( | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Self-review round one (implementation behavior): CLEAN Scope: Checked:
Findings: none.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Repair record (588085d), round one and round two applied to the repair Correction to round one: I wrote that each per-version
|
||
| "^(" + "|".join(re.escape(tag) for tag in _documented_tags) + ")$" | ||
| if _documented_tags | ||
| else r"^$" | ||
| ) | ||
|
|
||
| # Whitelist pattern for branches | ||
| smv_branch_whitelist = r"^master$" # Only build master branch | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,7 +35,7 @@ just docs lint | |
| just docs build | ||
| ``` | ||
|
|
||
| `just docs build` builds documentation from the configured Git refs with sphinx-multiversion. | ||
| `just docs build` builds `master`, the latest patch release of the newest minor versions (`SMV_MINOR_VERSIONS` in `docs/conf.py`), and the latest release of the previous major version if that tag contains `docs/conf.py`, with sphinx-multiversion. | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Independent review, expanded scope (relayed): FINDINGS (1 × P3), repaired; follow-up CLEAN
Covered: whitelist consumption, per-ref config loading and per-version subprocesses; numeric ordering, minor grouping, previous-major eligibility, maintenance releases, non-matching tags, and no tags; event priority, template resolution, switcher ordering, URLs, and current-version selection; the 404 redirect, the recipe, both workflows, and the changed text. Finding (P3), this line: the sentence promised an unconditional previous-major build. With today's tags the candidate is No functional defects. Non-actionable observations: local rebuilds keep stale output directories (pre-existing recipe behavior; CI builds from fresh checkouts). Local builds and docs-lint select from all matching tags; only the Docs deploy drops unpublished tags (noted in round one). Repair, checked from both self-review perspectives:
Independent follow-up: same reviewer and settings, session |
||
| To check the working tree, including uncommitted documentation changes, also run: | ||
|
|
||
| ```bash | ||
|
|
||
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.
Self-review round two (claims and operational behavior): FINDINGS, repaired
Scope: full pass over
git diff 645e04ebf9f818dfcc5336bd8af66adf1587f33a..9e3235e81fb5ed256e88f53cbe2ecec679ef54c1, the PR body, the commit message, and issue #868. The repair was then checked atcca5ad2e6d3a8a8940d8c52ffebdcfbdac3d44b8.Claims checked:
mastertook 45.6 s, so the 53 tags took about 964 s (16 min), plus about 80 s between builds. The PR body now says "about 16 minutes rebuilding all 53 released tags and 46 seconds onmaster". Issue Limit the multiversion docs build to the latest patch of the 3 newest minor versions #868 already stated "about 17 minutes of Sphinx time" and "masteralone took about 46 seconds", which is accurate.docs-lint.yamlrunsjust docs buildwithfetch-depth: 0. It has no unreleased-tag drop step, as noted in round one.buildjob for this PR (run 36331604190, head cca5ad2) ran 4 Sphinx builds (master, v3.34.0, v3.35.4, v3.36.0) and finished in 3 min 30 s. Recent docs-lint runs on other PRs took 19–20 min (36330087767, 36320484140, 36308386844). The Docs deploy workflow runs the same recipe, but its post-merge duration is not measured yet.sphinx_multiversion/git.py:copy_treeextractsgit archiveoutput, so the per-version trees have no.git.master/index.htmlhas exactly 4 options./vX.Y.Z/path, including a missing page of a built version, so the comment now says that. The PR body's WHAT bullet was aligned the same way.docs/testing.mdis the only prose that describesjust docs build.git grepfinds no hard-codedpyathena.dev/vX.Y.Zlinks in the repository.404.htmlcovers them. Its status is still 404, so search engines drop the old URLs. This PR does not verify that GitHub Pages serves the root404.htmlon the custom domain; that is listed as a post-merge check in TEST.Other findings: none.