Skip to content

feat(changelog): cite issues, PRs and discussions with Sphinx roles - #1008

Merged
JarryShaw merged 1 commit into
mainfrom
docs/719-changelog-citations
Oct 3, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
docs/719-changelog-citations

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 3, 2026 •

Copy link
Copy Markdown
Owner

Please follow the guide below

  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort)
  • make test passes, and a test case covers the change
  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible

A test case covers the change: 5 new tests in tests/project/test_changelog_md.py, each shown to fail without the fix. The make test box is left unticked deliberately — the full suite exhausts memory on the machine this was prepared on, so I ran tests/project/ and the docs build rather than claiming a green run I did not observe. CI's matrix is the authority on the rest.

What is the purpose of your pull request?

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • chore — anything else

Description of your pull request and other information

Part of #719, finishing what #989 settled for pcapkit/: a bare #NNN renders as plain text in the documentation, so 455 citations across docs/source/changelog/ become :issue:, :pr: and :discussion: roles — 434 in 1.5.0.rst over 253 distinct numbers, 21 across 13 older release notes. Every number's kind was resolved against the GitHub API rather than inferred: 267 issues, 186 pull requests (#30 and #121 among them) and 2 discussions (#106, #251).

The markup changes and the rendered text does not. Collapsing every role back to #NNN over the whole diff, with whitespace normalised, gives removed text identical to added text — 16 lines in the older notes, 356 in 1.5.0.rst. A separate scan asserts no role sits inside **bold**, *italic* or a literal, which renders as literal text with no Sphinx warning at all. The docs build stays at exit 0 and 61 warnings, with the warning lists line-for-line identical.

util/changelog_md.py had to learn the roles, which is why this is not a docs-only change. The root CHANGELOG.md is generated from these entries, and the generator's _RESIDUAL guard makes any role it cannot convert fatal — it knew only :rfc:. Unchanged, it rejects all 14 converted entries. It now converts the five extlinks roles from docs/source/conf.py into Markdown links, following sphinx.ext.extlinks for the link text and target, including the explicit-title form. The guard itself is untouched, so an unknown role such as :mod: is still fatal.

The role table is a module constant, not a run-time read of conf.py. MANIFEST.in prunes docs and then re-includes only docs/source/changelog/*.rst, so an sdist carries the converted entries and the generator but not the config; reading it live made a missing config a fatal ResidualMarkupError instead of a degradation. A test pins the constant against conf.py's extlinks and skips when docs/ is absent — drift-proof where it can be checked, correct where it cannot.

Left bare deliberately, and these are the only 3 bare citations left in the 14 files: #439 at 1.5.0.rst:708, where the literal text is the point and the number is still cited as :issue:439at `:1523` and `:1624`; and **two** of another project's numbers, JarryShaw/DictDumper#125 and JarryShaw/DictDumper#121 ``, at :1090 and `:1095`. `GH-nnn` would also have been left alone as this repository's alternative issue form, but that exclusion is vacuous here — there are 0 `GH-nnn` occurrences in any of these files or in `CHANGELOG.md`, so nothing was there to preserve.

Verification. tests/project/ gives 268 passed / 1 skipped / 864 subtests, and 59 passed / 1 skipped with conf.py moved aside. The regenerated CHANGELOG.md has an exact bijection with the source: 434 roles ↔ 434 links, 248 /issues/, 185 /pull/, 1 /discussions/, no number whose link text and target disagree.

* Converts 455 bare `#NNN` citations in `docs/source/changelog/` into
  `:issue:`, `:pr:` and `:discussion:` roles -- 434 in `1.5.0.rst`, 21
  across 13 older release notes. Each number's kind was resolved against
  the GitHub API, not guessed from context.
* Teaches `util/changelog_md.py` the five `extlinks` roles, so the
  generated `CHANGELOG.md` carries real Markdown links. Without this the
  generator rejects every converted entry: `_RESIDUAL` makes an
  unconvertible role fatal, and it knew only `:rfc:`.
* Holds the role table as a module constant rather than reading
  `docs/source/conf.py` at run time, because `MANIFEST.in` prunes `docs`
  and re-includes only the changelog entries -- an sdist has the entries
  but not the config. A test pins the constant against `conf.py` and
  skips when `docs/` is absent.
* Leaves bare: `GH-nnn`, citations inside literals, and another
  repository's numbers.

`tests/project/` passes 268 tests / 864 subtests, and 59 with `conf.py`
moved aside. Docs build unchanged at 61 warnings.
@JarryShaw JarryShaw added docs Pull requests that change documentation only (docs: subject prefix) feat Pull requests that add a new capability (feat: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one test Pull requests that add or correct tests (test: subject prefix) labels Oct 3, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 45b497ac3 — opus cross-review, a different model from the three sonnet agents that authored this. It confirmed all 8 claims by its own derivation and corrected two things in my description above, both now fixed.

Where it was stronger than my own verification. My nested-inline-markup proof was a per-line regex scan. It found 36 lines carrying **…** spans that wrap across source lines, which a per-line check cannot see, so it discarded that method and let docutils decide instead: since :issue:/:pr:/:discussion: are extlinks that plain docutils does not know, docutils emits one Unknown interpreted text role per string it actually parses as a role, and emits nothing for one swallowed by emphasis or a literal — which is exactly the silent defect. Result: 455 occurrences, 455 parsed as roles, 0 swallowed. That is a real proof where mine was suggestive.

Independently derived, not re-read: 274 distinct numbers resolved via GraphQL in batches — 152 issues, 120 pull requests, 2 discussions — 0 kind mismatches across all 455 occurrences, and no number cited under two kinds. Collapse-back equality per file, 372 lines each side. Necessity measured by reverting one line: exactly the 14 converted entries fail, no others. The sdist claim measured by running MANIFEST.in through setuptools' own FileList processor rather than reading it — 824 entries, docs/source/conf.py absent, the entries and the generator present.

My two errors it caught. The GH-nnn exclusion is vacuous — there are 0 GH-nnn occurrences in these files, so nothing was there to leave alone, and stating it as a checked exclusion invites the inference that such citations exist here. And there are two DictDumper citations, not the one I named. I verified both myself before accepting them.

Three non-blocking findings, recorded rather than fixed, since all are unexercised — every one of the 455 role contents is plain digits, and changing the diff would invalidate this verdict for a new head:

  1. extlink() omits the utils.unescape(text) that sphinx.ext.extlinks.make_link_role performs before splitting, so a reST-escaped content would carry its backslash into both text and URL. Everything else mirrors Sphinx faithfully, and _EXPLICIT_TITLE is explicit_title_re verbatim.
  2. The Markdown escaping is deliberately partial — a title holding *, _, backtick, < or \, or a URL holding <, > or ", would still mis-render. The code comment states the intent but not that the set is incomplete; one clause would pin it.
  3. INDEX points at docs/source/changelog.rst, which an sdist does not carry, so render()/main() cannot run from an unpacked sdist — only convert() and the shipped tests, which build their own index in a tempdir. Pre-existing and unchanged here, but it makes the EXTLINKS comment read slightly stronger than the sdist supports.

It did not run a real sphinx-build — docutils parsing stood in — so extlinks' own ExternalLinksChecker behaviour is unverified by it. CI's docs leg covers that.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 3, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Ready to merge at 45b497ac3. CI is complete and clean: 69 CheckRun legs green, 0 failed, 0 still running, 3 skipped (mergeStateStatus=CLEAN). The cross-review verdict above stands at this same head — nothing was pushed after it, so the review: good-to-go label still tracks the sha it was granted on.

The three skipped legs are the conditional ones that do not apply to this change, not suppressed failures.

Unpublished and unmerged, awaiting you.

@JarryShaw

Copy link
Copy Markdown
Owner Author

The one gap the cross-review left open is closed: a real Sphinx build ran on this PR and passed. It flagged that it had used docutils parsing rather than sphinx-build, so extlinks' own behaviour was unverified by it.

I checked which leg covers that, because Docs test gate reads SKIPPED here and that looked wrong on a docs-heavy change. It is not: that gate is the unit-test guard in front of a publish, and deploy-pages.yml:44 skips it on every pull request by design, whatever the content. The docs are built by the deploy-pages job instead — make -C docs html at deploy-pages.yml:118 — and only its Deploy step is conditioned off for a pull request, at :125. That job reports SUCCESS on 45b497ac3.

So the five roles render through real Sphinx with sphinx.ext.extlinks loaded, not only through a docutils stand-in. The other two skips are Gate (full suite, Python 3.14), gated on a gate-only workflow input that only a shipping workflow sets, and Compat Python 3.15 (scheduled), which runs on a schedule.

@JarryShaw
JarryShaw merged commit ecdc08a into main Oct 3, 2026
73 checks passed
@JarryShaw
JarryShaw deleted the docs/719-changelog-citations branch October 3, 2026 20:45
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 3, 2026
@JarryShaw JarryShaw added this to the 1.5 milestone Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Pull requests that change documentation only (docs: subject prefix) feat Pull requests that add a new capability (feat: subject prefix) test Pull requests that add or correct tests (test: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant