From 827b8665828e676607af20ab7d30cf230bf17f3a Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sun, 4 Oct 2026 18:10:23 -0400 Subject: [PATCH] docs(contributing): cut the contributing pages and repair drifted line references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Re-derives the `file:line` references into `.github/workflows/`, which had drifted by 50 to 200 lines. `create-release.yml`'s `unit-tests` job is at 148-155, not 142-149; `unit-tests.yml`'s `required-checks` is at 1141-1167 and its `changelog` job at 904-925. - Drops a pointer in `releasing.rst` saying the tag-push bypass is explained in the next section. That section is about skip cascades and never explained it, and the reason appears nowhere else in the subtree. - Cuts timed context per #719 — "today", "currently", "to date", "commented out since 2023", and a two-pass sweep narrative. Version-bounded breaking-change notes stay, including the one recording that four capture engines used to be in the `all` extra. - Concision across seven pages; four of the eleven are untouched. Citations are frozen: every `:issue:`/`:pr:`/`:discussion:` role, bare `#nnn` and issue URL is byte-identical per file. The `tests/`-and-changelog citation exemption and the paraphrase-the-ruling instruction both survive. `tests/project`: 268 passed, 1 skipped, 864 subtests passed. Part of #719. --- .../conventions/documentation.rst | 46 +++--- .../conventions/mint-criterion.rst | 2 +- .../contributing/conventions/process.rst | 8 +- .../conventions/registry-protocol.rst | 29 ++-- docs/source/contributing/pep.rst | 4 +- docs/source/contributing/releasing.rst | 139 ++++++++---------- docs/source/contributing/workflows.rst | 76 +++++----- 7 files changed, 140 insertions(+), 164 deletions(-) diff --git a/docs/source/contributing/conventions/documentation.rst b/docs/source/contributing/conventions/documentation.rst index 34fbb8085..a128671b3 100644 --- a/docs/source/contributing/conventions/documentation.rst +++ b/docs/source/contributing/conventions/documentation.rst @@ -29,41 +29,38 @@ and ``to`` stay lowercase unless they lead, which was the reading put to the own *Issue and Pull Request Labels* on :ref:`process` are both in it. **The casing is the easy half.** A heading can be in Title Case already and still -break the rule by being a clause rather than a title. The sweep of -:file:`docs/source/contributing/` under this ruling found, of 62 headings, 24 already -right, 16 needing only re-casing, and **22 to be rewritten**. *What a Failed Lookup -Raises* was correctly cased and is now *Failed-Lookup Exceptions*. +break the rule by being a clause rather than a title: *What a Failed Lookup Raises* was +correctly cased and is now *Failed-Lookup Exceptions*. Of the 62 headings under +:file:`docs/source/contributing/` when this was ruled, 22 needed rewriting and 16 only +re-casing. -The line that sweep drew, and the one to keep drawing: a **finite verb** makes a heading -a sentence and earns a rewrite, while a gerund or infinitive phrase is a noun phrase and -needs only re-casing -- so *Running the tests* became *Running the Tests* rather than -being reworded. +The line to draw: a **finite verb** makes a heading a sentence and earns a rewrite, +while a gerund or infinitive phrase is a noun phrase and needs only re-casing -- so +*Running the tests* becomes *Running the Tests* rather than being reworded. .. note:: A heading also has to describe the section beneath it, and a rewrite is the moment - to check that. Two of that sweep's own new titles were renamed again in review for - failing it. *One Approval, Whole Release* became *The Release Pipeline* because the - section opens on a Mermaid flowchart while the approval prose it had been named for - sat well below, leaving the page's most navigable artefact unfindable from the table - of contents. + to check that. *One Approval, Whole Release* was renamed *The Release Pipeline* + because the section opens on a Mermaid flowchart while the approval prose it had + been named for sat well below, leaving the page's most navigable artefact + unfindable from the table of contents. Renaming a Heading ~~~~~~~~~~~~~~~~~~ -Two mechanical traps, both of which bit on that sweep rather than being hypothetical. +Two mechanical traps, both of which have bitten rather than being hypothetical. **Re-measure the underline against the new text.** A case flip usually keeps the length; a rewrite almost never does, and docutils reports an underline shorter than -its heading in a build that does not fail on warnings. The sweep re-checked all 62 and -found none short, which is the standard to hold. +its heading in a build that does not fail on warnings. Re-check every heading; none +of the 62 was short. **A heading is a link target, so a rename is a repository-wide sweep** rather than a :file:`docs/source/` one. In scope: ``.rst`` prose, ``:ref:`` and ``:doc:`` link text, toctree entries, implicit ``` `Text`_ ``` references, **and** :mod:`pcapkit` docstrings -and the files under :file:`tests/`. Applying that scope to the heading rename took two -passes: the first reported one surviving stale reference; review found **five**, spread -across :file:`pcapkit/corekit/sentinels.py`, +and the files under :file:`tests/`. A heading rename once left **five** stale +references, spread across :file:`pcapkit/corekit/sentinels.py`, :file:`tests/corekit/test_sentinel_exports_unit.py` and :file:`tests/project/test_conventions_doc_claims.py`. The first of those is a shipped module docstring that renders into the API reference, so a reader following it searches @@ -73,9 +70,8 @@ the page for a string no longer on it. **Derive the list of old headings from the pre-change file, not from the diff.** A diff-derived list silently drops any heading whose underline is not adjacent in the - hunk, which is why the second pass undercounted as well as the first. What - worked was a whitespace-flattened search for every pre-change heading string across - every tracked file. + hunk. A whitespace-flattened search for every pre-change heading string across + every tracked file is what works. Nothing in CI catches a reference a rename left behind. :file:`docs/source/conf.py` sets no ``nitpicky`` and :file:`docs/Makefile` leaves ``SPHINXOPTS`` empty, so the @@ -121,7 +117,7 @@ type hierarchy. **Quote a node label whose text Mermaid would otherwise try to p and leave a bare identifier bare. That is what the exemplars do with *node* labels, and it falls close to the ``TD``/``LR`` line without following it: every node label in the six ``TD`` graphs is quoted, since every one of them is prose; the ``LR`` graphs quote -only where the text forces it, which today is ``h1["HTTP/1.*"]`` and ``h2["HTTP/2"]`` in +only where the text forces it, which is ``h1["HTTP/1.*"]`` and ``h2["HTTP/2"]`` in :file:`docs/source/pcapkit/protocols/index.rst` and nowhere else. Everything else there uses the bare ``A{{Meta}}``, ``B(Base)``, ``D([user customisation ...])`` and ``subgraph name [Title]`` forms, and spends its quotes on ``click`` targets instead. @@ -170,7 +166,7 @@ is the duplication the owner ruled out on :issue:`719`. For the same reason, do not add an inline ``.. contents::`` to a page. The ``furo`` theme already renders a sticky page-local table of contents on every page, so an inline directive is a second copy of it; the one in :file:`docs/source/index.rst` - has been commented out since 2023 and should stay that way. + is commented out and should stay that way. One setting there was decided by measurement rather than by preference: ``toc_object_entries`` carries, in a comment beside it, the figures that settled it, @@ -292,7 +288,7 @@ Format and Mechanics * **reStructuredText under** :file:`docs/source/`, **Markdown outside it.** The owner ruled this on :issue:`719`, correcting a blanket *always* ``.rst`` that had been in - circulation until then: the Sphinx documentation is reST, and the other documents -- + circulation: the Sphinx documentation is reST, and the other documents -- the READMEs included -- are Markdown where that applies. ``CONTRIBUTING.md``'s own *Documentation* section records the same split. One trap arrived with the ruling: :file:`MANIFEST.in` reaches the two READMEs under :file:`examples/` through diff --git a/docs/source/contributing/conventions/mint-criterion.rst b/docs/source/contributing/conventions/mint-criterion.rst index 95eb74d0b..2b9c0cb23 100644 --- a/docs/source/contributing/conventions/mint-criterion.rst +++ b/docs/source/contributing/conventions/mint-criterion.rst @@ -143,6 +143,6 @@ only ``ast.Name`` nodes finds every mint and no unmint at all: Calling ``Cls(value)`` on a registry whose ``_missing_`` mints **mutates the class**. A probe is not a read: it installs a member that every later lookup then finds. :class:`~pcapkit.const.mh.cga_type.CGAType` is the one registry this applies - to today, but snapshot ``{member.value for member in Cls}`` before any lookup + to, but snapshot ``{member.value for member in Cls}`` before any lookup regardless, and use a throwaway process per registry when comparing behaviour across revisions. diff --git a/docs/source/contributing/conventions/process.rst b/docs/source/contributing/conventions/process.rst index faa96d77e..f0ccb9a2e 100644 --- a/docs/source/contributing/conventions/process.rst +++ b/docs/source/contributing/conventions/process.rst @@ -18,7 +18,7 @@ The ``all`` Extra ``all`` means **core addons only** -- the things that let the library itself run at full functionality -- rather than everything a user might conceivably want. The owner settled that on :issue:`910` and named -the three that qualify to date: the CLI addon, the crypto addon, and ``pycrate``. +the three that qualify: the CLI addon, the crypto addon, and ``pycrate``. On the tree, in :file:`pyproject.toml`: @@ -226,9 +226,9 @@ these, so that its status is readable without opening it: Two things the board shows rather than the rule: ``wip`` and ``needs: decision`` legitimately **co-occur**, when the bulk of an issue is being worked and one -sub-question is held for the owner -- :issue:`918` itself was labelled that way while this -page was being written. And an open issue with no state label at all is a gap rather -than a category, which is worth checking for rather than assuming away: +sub-question is held for the owner -- :issue:`918` itself was labelled that way. And +an open issue with no state label at all is a gap rather than a category, which is +worth checking for rather than assuming away: .. code-block:: shell diff --git a/docs/source/contributing/conventions/registry-protocol.rst b/docs/source/contributing/conventions/registry-protocol.rst index 803759b15..ce8a29048 100644 --- a/docs/source/contributing/conventions/registry-protocol.rst +++ b/docs/source/contributing/conventions/registry-protocol.rst @@ -30,9 +30,9 @@ bespoke ``__new__`` exempts a registry from the template, not from the protocol. The Two-Tier Hierarchy ~~~~~~~~~~~~~~~~~~~~~~ -:class:`~pcapkit.corekit.enum.EnumRegistry` is not the only base any more. Since -phase 1 of :issue:`877` it has a -parent, and the line between them is whether the enumeration may *grow*: +:class:`~pcapkit.corekit.enum.EnumRegistry` is not the only base: per phase 1 of +:issue:`877` it has a parent, and the line between them is whether the enumeration +may *grow*: =================================================== ============================================================== :class:`~pcapkit.corekit.enum.EnumLookup` ``get``, ``get_all``, ``_validate_value`` @@ -65,7 +65,7 @@ itself and broken ``int``, ``str`` and flag registries at once. *instead* of ``register``, and it answers the owner's other requirement: the base has to offer range-validation logic for its inheriting classes to hook into. The base implementation accepts everything; an override states a range, in the shape the -generated registries currently spell by hand in ``_missing_``: +generated registries spell by hand in ``_missing_``: .. code-block:: python @@ -104,10 +104,9 @@ Three things about it are easy to get wrong: Re-parenting every non-registry enumeration onto :class:`~pcapkit.corekit.enum.EnumLookup` was **phase 2** of - :issue:`877`, and it is now - **complete**: the phase landed for 24 of the 24 non-registry enumerations. - **Zero enumerations remain outside the hierarchy**, measured by the same runtime - walk over both the :mod:`enum` and ``aenum`` flavours that once found seven. + :issue:`877`, and it is **complete**: the phase landed for 24 of the 24 + non-registry enumerations. **Zero enumerations remain outside the hierarchy**, + measured by a runtime walk over both the :mod:`enum` and ``aenum`` flavours. Failed-Lookup Exceptions ~~~~~~~~~~~~~~~~~~~~~~~~ @@ -273,7 +272,7 @@ in **zero** of them, and neither class carried an alias (``__members__`` 6 and 4 docstrings to refer to. Offered the choice between merging the widened copies and deleting them in a follow-up, the owner ruled for deleting them outright. Both ``get`` methods and both suppressions went with it, and -:mod:`pcapkit.protocols.internet.mh` now defines no ``get`` at all. +:mod:`pcapkit.protocols.internet.mh` defines no ``get`` at all. The one input where the copies **did** differ is why this matters beyond line count, and it is the trap for whoever writes the next override: the base branches on @@ -476,7 +475,7 @@ That leaves the classes with something to decide: exactly as for the :class:`int` tier above; ``PDUKind``'s are :class:`str` ASN.1 identifiers from 3GPP TS 38.413, and ASN.1 identifiers are case-significant by construction, so it lands the same way for a different reason. - **None** of the six defines a ``get`` of its own any more. Two did when this + **None** of the six defines a ``get`` of its own. Two did when this audit was taken -- ``FastBindingAcknowledgmentStatus`` and ``IPv6AddressPrefixCode``, for signature reasons (no ``default``, and an :class:`int`/:class:`str` dispatch) @@ -519,7 +518,7 @@ wider than a case fix: * ``CommandType`` **and** ``ConformanceRequirement``. By parity with ``TransportProtocol`` -- an :class:`int`-valued enumeration whose *names* are the specification's own tokens -- the measured spelling disagreement above would make - these two case-insensitive. Nothing looks them up by string today, though: the + these two case-insensitive. Nothing looks them up by string, though: the crawler translates the CSV's lower-case letters to the upper-case member names at generation time. Both classes inherit :class:`~pcapkit.corekit.enum.EnumLookup`, since @@ -540,11 +539,9 @@ rather than changing it. .. note:: - The obstacle this page used to record -- that the base's string-key path does not - fall through to a value lookup, so an ``aenum.StrEnum`` registry would stop - resolving a valid value that is not also a name -- **no longer applies.** - :meth:`~pcapkit.corekit.enum.EnumLookup.get` now checks ``_value2member_map_`` - when the name lookup misses, so such a value resolves: + :meth:`~pcapkit.corekit.enum.EnumLookup.get` checks ``_value2member_map_`` when + the name lookup misses, so an ``aenum.StrEnum`` registry still resolves a valid + value that is not also a name: .. code-block:: pycon diff --git a/docs/source/contributing/pep.rst b/docs/source/contributing/pep.rst index 80dcecde5..6071dbf80 100644 --- a/docs/source/contributing/pep.rst +++ b/docs/source/contributing/pep.rst @@ -611,7 +611,7 @@ Writing the reassembler is the smaller half of the job. It would go at methods, :meth:`~pcapkit.foundation.reassembly.reassembly.Reassembly.reassembly` and :meth:`~pcapkit.foundation.reassembly.reassembly.Reassembly.submit`. Registering -it, though, is currently inert: +it, though, is inert: :meth:`~pcapkit.foundation.extraction.Extractor.register_reassembly` accepts any protocol name, while :class:`~pcapkit.foundation.extraction.Extractor` only ever instantiates the @@ -996,7 +996,7 @@ dissectors later. The third format, the :rfc:`6088` traffic selectors of nobody has claimed it. **Wave 2 or 3 — per-entry coverage of the option and parameter registries.** The -option, parameter, block and frame registries are *enumerated* today but not +option, parameter, block and frame registries are *enumerated* but not *implemented* throughout: a code can be registered, and named, and still have no working per-entry read/make pair behind it. The goal of this item is to close that gap to zero for every code whose format is actually specified. diff --git a/docs/source/contributing/releasing.rst b/docs/source/contributing/releasing.rst index 386a8cef7..f0949faaa 100644 --- a/docs/source/contributing/releasing.rst +++ b/docs/source/contributing/releasing.rst @@ -39,31 +39,28 @@ The same script is also the manual entry point: run release that is not a vendor-registry refresh -- a feature or a fix that has landed and is ready to go out. -Editing ``__version__`` by hand instead, without also moving -:file:`CITATION.cff`'s ``version`` field to match, is **not** a safe -shortcut -- it does not drift through silently, it blocks the release. +Editing ``__version__`` by hand without also moving :file:`CITATION.cff`'s +``version`` field to match is **not** a safe shortcut: it blocks the release. ``tests/project/test_bump_version.py``'s ``RepositoryCitationTests.test_the_citation_file_names_the_packaged_version`` -(``:493-513``) asserts the two agree, and that assertion is not optional on -the release path: ``create-release.yml``'s ``unit-tests`` job -(``:142-149``) calls ``unit-tests.yml`` with ``gate-only: true``, which runs -the **full** suite rather than the tiered subset an ordinary push runs -(its ``gate`` job, ``unit-tests.yml:734-803``). ``version_check`` itself -depends on that job (``needs: [ unit-tests ]``, ``:154``), so a citation left -behind fails the gate before ``version_check`` ever runs, and well before any -approval is requested. Either run the script, or move both fields by hand in -the same commit. +(``:494-514``) asserts the two agree, and ``create-release.yml``'s +``unit-tests`` job (``:148-155``) calls ``unit-tests.yml`` with +``gate-only: true``, which runs the **full** suite rather than the tiered +subset an ordinary push runs (its ``gate`` job, ``unit-tests.yml:931-1000``). +``version_check`` depends on that job (``needs: [ unit-tests ]``, ``:160``), so +a citation left behind fails the gate before ``version_check`` runs, and well +before any approval is requested. Either run the script, or move both fields by +hand in the same commit. This is the **only** edit a person makes to get a release started -- the tag, the Release, and every upload are the workflow's job from here. Two other -checks run unconditionally on the same path and are worth knowing about -rather than being surprised by, though neither is a step in *this* process: -the ``changelog`` job (``unit-tests.yml:707-728``) fails outright if +checks run unconditionally on the same path, though neither is a step in *this* +process: the ``changelog`` job (``unit-tests.yml:904-925``) fails outright if ``CHANGELOG.md`` has drifted from its source entry under :file:`docs/source/changelog/`, and the release-body step warns, without failing, if that entry's heading still reads "unreleased" -(``create-release.yml:395-396``). Both are enforcing work the changelog -convention already asks for, not extra work this process adds. +(``create-release.yml:402-403``). Both enforce what the changelog convention +already asks for. Two Ways to Start a Release --------------------------- @@ -75,11 +72,11 @@ neither is a workaround for the other: ``push: tags: ['v*']`` Pushing a tag matching ``v*`` yourself. This chooses **which commit** to release, not which version: ``version_check``'s checkout - (``:158-161``) carries no explicit ``ref:``, so it checks out whatever + (``:164-167``) carries no explicit ``ref:``, so it checks out whatever commit the pushed tag points at, and reads ``pcapkit.__version__`` out of - *that* tree (``:178``). Every downstream tag and release name is built + *that* tree (``:184``). Every downstream tag and release name is built from that output -- ``tag_name: "v${{ needs.version_check.outputs.PCAPKIT_VERSION }}"`` - appears at ``:419``, ``:597`` and ``:837``, always from + appears at ``:425``, ``:603`` and ``:843``, always from ``PCAPKIT_VERSION``, never from ``github.ref_name`` -- so the version actually released is whatever ``__version__`` says on the tagged commit, not the name of the tag that was pushed. The two agree only if the tag @@ -118,7 +115,7 @@ using "the ``v*`` tag exists" as a proxy for "this version was already published". Those diverge exactly when a release half-completes -- ``github`` tags, a later job fails or is rejected, and the tag is left behind with the publish incomplete -- which is the whole subject of `Precautions`_ below. -``github`` still uses that guard for its own artefact today, since the tag it +``github`` still uses that guard for its own artefact, since the tag it checks *is* the one it is about to create: .. code-block:: yaml @@ -145,10 +142,9 @@ All three evidence checks run inside ``version_check``, curling PyPI's and Anaconda's public, unauthenticated JSON APIs; the per-leg check runs inside ``conda`` itself, once per matrix leg, against the same Anaconda endpoint. -The two triggers still hit these guards the same way as before: on the -tag-push path ``startsWith(github.ref_name, 'v')`` unconditionally bypasses -the evidence check, for the reasons in the next section below; on the -``workflow_run`` path it falls through to whichever evidence that job owns. +On the tag-push path ``startsWith(github.ref_name, 'v')`` bypasses the +evidence check unconditionally; on the ``workflow_run`` path it falls through +to whichever evidence that job owns. The Skip-Cascade Guard ---------------------- @@ -213,20 +209,19 @@ The Release Pipeline class GH gate A transitive reduction: ``version_check`` is also a direct ``needs:`` of -``tag``, ``pypi`` and ``conda`` in the file itself, alongside the edges drawn -above -- omitted here because ``github`` already implies it (``github`` -itself depends on ``version_check``), and drawing it would add three more +``tag``, ``pypi`` and ``conda`` in the file, but ``github`` already implies it +(``github`` itself depends on ``version_check``), so drawing it would add three lines without changing which job can start before which. ``release_status``'s -edges are drawn in full, since unlike the others it depends directly on -*every* job precisely so that it can run regardless of which of them skipped. +edges are drawn in full, since it depends directly on *every* job precisely so +that it can run regardless of which of them skipped. -Only ``github-release`` carries a required reviewer today; ``conda-tag``, +Only ``github-release`` carries a required reviewer; ``conda-tag``, ``pypi`` and ``anaconda`` kept their environments but had their reviewers removed on `#887 `__. What still makes one approval mean *the whole release* is the ``needs:`` graph: ``tag`` and ``pypi`` both depend on ``github`` -(``.github/workflows/create-release.yml:443,521``), and ``conda`` depends on -``tag`` and ``github`` (``:620``) -- so nothing downstream of ``github`` can +(``.github/workflows/create-release.yml:449,527``), and ``conda`` depends on +``tag`` and ``github`` (``:626``) -- so nothing downstream of ``github`` can start before it is approved, and rejecting it leaves nothing tagged and nothing published. Before this change ``tag`` depended on ``version_check`` alone, so removing its reviewer without moving this dependency would have let @@ -247,34 +242,32 @@ Precautions `Per-Job Evidence, Not a Shared Proxy`_ above is what prevents it: ``tag``, ``pypi`` and ``conda`` each check whether - *their own* artefact is missing rather than whether the ``v*`` tag exists, so an incomplete - release runs the jobs that did not finish instead of skipping them. A - half-finished release now self-heals on the next ``workflow_run``-triggered + *their own* artefact is missing rather than whether the ``v*`` tag exists, so an + incomplete release runs the jobs that did not finish instead of skipping them. + A half-finished release now self-heals on the next ``workflow_run``-triggered attempt, or on re-running the workflow by hand -- see `Recovery`_ below. ``release_status`` is the other half: it runs unconditionally and reports, with a ``::notice``, a ``::warning`` or a failing ``::error``, why a run - released nothing or that it released something -- so a stranded release is - never only a green checkmark with no explanation, even if the evidence - checks above somehow disagree with reality. - -**Do not hand-make a tag to route around a stuck release.** This is a -narrower rule than "never tag by hand" -- pushing a *fresh* ``v`` -tag to start a release, as in `Two Ways to Start a Release`_ above, is fine -and unchanged by any of this. What is not sanctioned is deleting and -re-pushing a tag that a stuck run already created, to force a retry. A -``git push`` of a tag ref that already points at the same commit produces no -new event (documented GitHub Actions behaviour, not measured here), so the -only way to make that push fire again is to delete the tag first -- which -both touches a ref the release automation owns, and lands the retry back on -the tag-push path, where every job's guard is unconditionally bypassed (see -`Two Ways to Start a Release`_ above) regardless of what has already gone -out. Neither is worth the risk of a double upload to an index that cannot -take one back -- and it is no longer necessary either, since a plain re-run -now self-heals; see `Recovery`_ below. + released nothing or that it released something, so a stranded release is + never only a green checkmark, even if the evidence checks disagree with + reality. + +**Do not hand-make a tag to route around a stuck release.** This is narrower +than "never tag by hand": pushing a *fresh* ``v`` tag to start a +release, as in `Two Ways to Start a Release`_, is fine. What is not sanctioned +is deleting and re-pushing a tag that a stuck run already created, to force a +retry. A ``git push`` of a tag ref that already points at the same commit +produces no new event (documented GitHub Actions behaviour, not measured +here), so the push only fires again after the tag is deleted -- which touches a +ref the release automation owns, and lands the retry back on the tag-push path, +where every job's guard is bypassed regardless of what has already gone out. +Neither is worth the risk of a double upload to an index that cannot take one +back, and neither is needed, since a plain re-run now self-heals; see +`Recovery`_ below. **``environment: pypi`` stays even though its reviewer is gone.** ``pypi`` publishes through PyPI's OIDC trusted publishing (``environment: pypi`` at -``:516``, ``id-token: write`` at ``:520``), and PyPI's trusted-publisher +``:522``, ``id-token: write`` at ``:526``), and PyPI's trusted-publisher configuration can be scoped to a GitHub Actions environment name. If this project's publisher on PyPI is scoped that way, removing the ``pypi`` name -- not just its reviewer -- would break the upload with a claim mismatch; @@ -285,7 +278,7 @@ the name regardless, since there is no upside to removing it. **``conda-tag`` writes to ``main``, so a release is not read-only on the branch.** The ``tag`` job resets :file:`conda/build` to ``0``, commits that, and pushes straight to ``main`` -(``.github/workflows/create-release.yml:461-481``) before cutting the +(``.github/workflows/create-release.yml:467-487``) before cutting the ``conda-+0`` tag. Approving ``github-release`` therefore also approves a commit landing on the default branch, not only the artefacts that sound like they are the point. @@ -294,29 +287,27 @@ Recovery -------- The sanctioned recovery from a failed or partial release run is re-running -the workflow -- nothing more elaborate, and specifically not re-tagging by -hand, for the reasons above. This is now the actual fix rather than a -best-effort suggestion: `Per-Job Evidence, Not a Shared Proxy`_ above means -``tag``, ``pypi`` and ``conda`` each check -whether *their own* artefact is missing, so a re-run finishes whichever jobs -did not complete last time instead of skipping them on the ``v*`` tag's mere +the workflow, not re-tagging by hand, for the reasons above. This is the +actual fix rather than a best-effort suggestion: `Per-Job Evidence, Not a +Shared Proxy`_ above means ``tag``, ``pypi`` and ``conda`` each check whether +*their own* artefact is missing, so a re-run finishes whichever jobs did not +complete last time instead of skipping them on the ``v*`` tag's mere existence. ``pypi`` was always safe to re-run (``skip-existing: true``); ``conda`` now is too, because each matrix leg checks Anaconda for its own platform/Python distribution before uploading and skips only that leg's upload if it is already there. -**A re-run no longer needs to be verified by hand against the PyPI and -Anaconda listings, because ``release_status`` does it for you.** That job -runs on every ``Create Release`` attempt regardless of what else skipped, and -reconciles each of ``github``/``tag``/``pypi``/``conda`` *against its own -evidence* rather than demanding the same outcome from all four -- a target -counts as reconciled if it actually succeeded, or if it skipped *because* its -own evidence already said it was done. This distinction is what makes a -self-heal reportable at all: retrying after a partial release is supposed to -produce a **mixed** result, ``github``/``tag`` skipping because their -artefacts already exist while ``pypi``/``conda`` run and finish what did not -complete last time, and a check that instead demanded uniformity across all -four would read that legitimate mix as the failure it is trying to detect. +**A re-run does not need to be verified by hand against the PyPI and Anaconda +listings, because ``release_status`` does it.** That job runs on every +``Create Release`` attempt regardless of what else skipped, and reconciles +each of ``github``/``tag``/``pypi``/``conda`` *against its own evidence* +rather than demanding the same outcome from all four -- a target counts as +reconciled if it succeeded, or if it skipped *because* its own evidence +already said it was done. That is what makes a self-heal reportable: a retry +after a partial release legitimately produces a **mixed** result +(``github``/``tag`` skipping because their artefacts exist while +``pypi``/``conda`` finish the rest), and a check demanding uniformity would +read that mix as the failure it exists to detect. With that reconciliation, ``release_status`` reports exactly one of: nothing to release because the trigger had nothing to do (quiet ``::notice``), diff --git a/docs/source/contributing/workflows.rst b/docs/source/contributing/workflows.rst index 8a44fd14f..94844814a 100644 --- a/docs/source/contributing/workflows.rst +++ b/docs/source/contributing/workflows.rst @@ -6,10 +6,9 @@ GitHub Actions Workflows The eight workflows under :file:`.github/workflows/` trigger each other, and the chain that results is not visible from any single file -- reading one ``on:`` block never shows what a *different* workflow's completion goes on - to start. This page is the repository-wide graph. It does not - cover the release *pipeline* itself -- the job-by-job path from a version - bump to a published package -- which :doc:`releasing` already documents in - full; this page cross-references that rather than duplicating it. + to start. This page is the repository-wide graph. The release *pipeline* + itself, job by job from a version bump to a published package, is on + :doc:`releasing`. At a Glance ----------- @@ -194,7 +193,7 @@ the caller's own workflow run, as one of the caller's own jobs, rather than starting an independent run after the caller finishes. All four calls in this repository target the same reusable workflow, with the same input: -* ``.github/workflows/create-release.yml:101`` -- job ``unit-tests`` calls +* ``.github/workflows/create-release.yml:153`` -- job ``unit-tests`` calls ``./.github/workflows/unit-tests.yml`` with ``gate-only: true``. * ``.github/workflows/cron-conda.yml:43`` -- job ``unit-tests`` calls ``./.github/workflows/unit-tests.yml`` with ``gate-only: true``. @@ -215,7 +214,7 @@ would test the same commit several times over -- and ``required-checks`` (see `Required Status Checks`_ below), gated out by its own ``if:`` rather than by having already run. ``changelog`` carries no ``if:`` at all and runs on every path regardless -- deliberately, per its own -comment (``unit-tests.yml:695-701``): ``create-release.yml`` feeds +comment (``unit-tests.yml:892-898``): ``create-release.yml`` feeds ``CHANGELOG.md`` to the GitHub Release body, so the release path is exactly where a drifted file must not go unchecked. Confirmed on run `36210743295 `__ (a @@ -236,18 +235,18 @@ carries: if: ${{ github.event_name != 'workflow_run' || github.event.workflow_run.conclusion == 'success' }} -(``create-release.yml:98``). On the ``workflow_run`` path, this evaluates +(``create-release.yml:150``). On the ``workflow_run`` path, this evaluates **false**, and the job is skipped outright, whenever the upstream Vendor Update run's own conclusion was anything other than ``success`` -- including ``skipped``, which is exactly what Vendor Update's *own* jobs report when *their* ``workflow_run``-path guards decline (see ``cron-vendor.yml``'s ``vendor-update`` job). ``version_check`` then depends on that job with a -plain ``needs: [ unit-tests ]`` (``create-release.yml:108``) and no +plain ``needs: [ unit-tests ]`` (``create-release.yml:160``) and no ``always()`` override, so a skipped dependency skips it too -- and every job past it is gated the same way: ``github`` needs ``[ version_check ]`` -(``:156``), ``tag`` needs ``[ github, version_check ]`` (``:251``), ``pypi`` -needs ``[ github, version_check ]`` (``:319``), and ``conda`` needs -``[ tag, github, version_check ]`` (``:406``). +(``:348``), ``tag`` needs ``[ github, version_check ]`` (``:449``), ``pypi`` +needs ``[ github, version_check ]`` (``:527``), and ``conda`` needs +``[ tag, github, version_check ]`` (``:626``). .. mermaid:: @@ -271,16 +270,12 @@ needs ``[ github, version_check ]`` (``:319``), and ``conda`` needs classDef skipped fill:#fdecea,stroke:#c0392b,stroke-dasharray:4 2 class UTX,VC,GH,TAGJ,PY,CD skipped -This is a transitive reduction, the same choice :doc:`releasing` makes for -the identical fan-out: ``version_check`` is also a direct ``needs:`` of -``tag`` and ``pypi`` in the file itself (``create-release.yml:251,319``), on -top of the ``github`` edge drawn above, and ``conda`` needs ``version_check`` -directly too (``:406``) alongside ``tag`` and ``github``. All three are -omitted here for the reason :doc:`releasing` gives for its own copy of this -same graph: ``github`` already implies them, since ``github`` itself depends -on ``version_check``, and drawing all of ``version_check``'s direct -dependents would add lines without changing which job can start before -which, or which job a skip in ``version_check`` reaches. +This is a transitive reduction, as in :doc:`releasing`: ``version_check`` is +also a direct ``needs:`` of ``tag`` and ``pypi`` (``create-release.yml:449,527``) +and of ``conda`` (``:626``), but ``github`` already implies those edges, since +it depends on ``version_check`` itself. Drawing them would add lines without +changing which job can start before which, or which job a skip in +``version_check`` reaches. The whole run then reports **skipped**, not failed -- there is no red X to notice. Observed in production on runs `36511610205 @@ -304,8 +299,8 @@ Required Status Checks Ruleset ``23497679`` on ``main`` requires six contexts, with ``strict_required_status_checks_policy: true`` (a branch must be up to date -with ``main`` before it can merge, not merely green). Found by grepping every -workflow file for each name rather than assuming the obvious one: +with ``main`` before it can merge, not merely green). Each was found by +grepping every workflow file for its name: .. list-table:: :header-rows: 1 @@ -315,7 +310,7 @@ workflow file for each name rather than assuming the obvious one: - Where * - ``Required checks passed`` - job ``required-checks`` - - ``unit-tests.yml:945`` + - ``unit-tests.yml:1142`` * - ``Compat Python 3.10`` - job ``compatibility``, matrix leg ``3.10`` - ``python-compatibility.yml:31,41`` @@ -332,20 +327,17 @@ workflow file for each name rather than assuming the obvious one: - job ``compatibility``, matrix leg ``3.14`` - ``python-compatibility.yml:31,45`` -``grep -rn "Required checks passed" .github/workflows/*.yml`` turns up -exactly one job carrying that literal ``name:`` (``unit-tests.yml:945``). -``grep -rn "Compat Python" .github/workflows/*.yml`` turns up two job -``name:`` lines in ``python-compatibility.yml`` -- the required -``compatibility`` job's template (``:31``, ``Compat Python ${{ -matrix.python-version }}``, expanding to the five required legs at -``:41-45``) and the separate, non-required ``compatibility-nightly`` job -(``:69``, ``Compat Python 3.15 (scheduled)``, cited again below for its own -``continue-on-error`` policy) -- plus one comment in ``unit-tests.yml`` -(``:806``) that references the string without defining it. +``Required checks passed`` is defined by exactly one job +(``unit-tests.yml:1142``). ``Compat Python`` is defined by two in +``python-compatibility.yml``: the required ``compatibility`` job (``:31``, +``Compat Python ${{ matrix.python-version }}``, expanding to the five required +legs at ``:41-45``) and the non-required ``compatibility-nightly`` job (``:69``, +``Compat Python 3.15 (scheduled)``, see below). A comment in +``unit-tests.yml`` (``:1003``) mentions the string without defining it. ``Required checks passed`` is itself an aggregate, not a single check run -- but it stands in for **17** of the ruleset's originally-named 22 contexts, -not all 22. ``unit-tests.yml``'s own comment (``:805-826``) accounts for the +not all 22. ``unit-tests.yml``'s own comment (``:1002-1023``) accounts for the 22: five each for ``test`` and ``integration``, five for the live ``Compat Python 3.10``-``3.14`` contexts (emitted by ``python-compatibility.yml``, a *different* workflow file -- nothing in ``unit-tests.yml`` could ever stand @@ -356,11 +348,11 @@ stopped producing once it became a Python x engine matrix, and two for ``test``/``integration``/dead-``Engines``/``pypcap-parity`` slots -- 5 + 5 + 5 + 2 = 17 -- via its own ``needs: [test, integration, engine-tests, pypcap-parity]`` plus an explicit per-dependency check -(``unit-tests.yml:944-970``). The five ``Compat`` contexts stay required +(``unit-tests.yml:1141-1167``). The five ``Compat`` contexts stay required exactly as they are and are listed separately in the table above. This job runs on Unit Tests' own ``push``/``pull_request`` triggers; the ``gate-only: true`` reusable calls documented above skip it via -``if: ${{ always() && inputs.gate-only != true }}`` (``:947``), so it is never +``if: ${{ always() && inputs.gate-only != true }}`` (``:1144``), so it is never produced -- and never expected -- on those paths. ``Compat Python 3.10``-``3.14`` are five ordinary matrix legs, not an @@ -378,16 +370,16 @@ Deployment ``environment:`` Four jobs, all in ``create-release.yml``, declare a deployment ``environment:`` and can therefore pause for an approval: -* ``github`` -- ``environment: github-release`` (``create-release.yml:153``) -* ``tag`` -- ``environment: conda-tag`` (``create-release.yml:249``) -* ``pypi`` -- ``environment: pypi`` (``create-release.yml:314``) -* ``conda`` -- ``environment: anaconda`` (``create-release.yml:403``) +* ``github`` -- ``environment: github-release`` (``create-release.yml:345``) +* ``tag`` -- ``environment: conda-tag`` (``create-release.yml:447``) +* ``pypi`` -- ``environment: pypi`` (``create-release.yml:522``) +* ``conda`` -- ``environment: anaconda`` (``create-release.yml:623``) No other workflow in scope here declares one -- including ``deploy-pages.yml``, whose ``deploy-pages`` job pushes straight to the ``gh-pages`` branch via ``JamesIves/github-pages-deploy-action`` rather than through an ``environment:``-gated deployment. Of the four above, only -``github-release`` carries a required reviewer today; :doc:`releasing` +``github-release`` carries a required reviewer; :doc:`releasing` covers why one approval on that single environment is enough to gate the whole release graph, and is the page to read for the reasoning rather than repeating it here.