From bd73c99278c96dd87908430dadfab924422df478 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Thu, 1 Oct 2026 18:06:43 -0400 Subject: [PATCH 1/9] docs(pcapkit,ci): cite the issue a defect belongs to, not the pull request (#719) Per the owner's ruling on #719, replace every reference to a pull-request number in pcapkit/** and .github/workflows/** comments and docstrings with the issue it closed, or a description where no issue covers it. - 92 real PR citations in pcapkit/ (93 was the estimate; the gap is RFC packet-diagram and hex-format-spec false positives, plus one cross-repo issue citation that only coincidentally matched a PyPCAPKit PR number). - 13 PR citations in .github/workflows/, matching the estimate exactly. - Several citations named two or three numbers for one claim where a PR closed several issues, or several PRs closed the same issue; deduplicated rather than left reading "#425 and #425". Four review rounds caught the same category error recurring: several sites had relocated a verbatim quote or a specific finding into the issue number rather than describing where the ruling was actually given, so the quote no longer existed where the sentence pointed. Fixed each by naming the issue while locating the ruling honestly -- "a ruling given in review of the work for #N" -- the same shape already used on this repo's conventions docs. Two sites needed the inverse correction instead: the #923 quote in enum.py/exceptions.py genuinely is recorded on #923's own thread, just attributed there to the pull request that implemented it, so those read "a ruling recorded on GitHub issue #923" rather than pointing elsewhere. Also fixed a lost conjunction and an ordinal/number mismatch in corekit/enum.py, a self-contradicting below/above pointer repeated across three internet/ files, and a number collision in http.py where one issue ended up naming both a defect and the change that closed it. Final sweep: grepped the whole tree for the word "verbatim" -- the marker that makes a quote-attribution claim falsifiable -- across all 30 files under pcapkit/ that carry it, and checked every quote this way names against the actual issue thread. Caught two more of the same defect: vendor/__main__.py's #872 citation (the quote is in the implementing pull request's review, not #872 itself) and four sites across mh.py attributing to #935 a ruling that only exists in the review of the pull request that implemented it -- #935's own thread holds just the superseded widen-not-delete proposal. Both fixed the same way. Every other quote-bearing claim the sweep found -- #911, #937, three distinct #877 quotes, both #842 quotes, and the #860/#808/#806/#886/#917 rewrites from earlier in this pass -- resolves to the thread it names. Verified: targeted pytest across every touched module passes, including the test that pins the vendor/const apptype.py get() region as byte-identical, reconfirmed after each amendment. Both edited workflow YAML files parse before and after with unchanged key counts. --- .github/workflows/lint.yml | 4 +- .github/workflows/unit-tests.yml | 19 ++--- pcapkit/const/ftp/command.py | 2 +- pcapkit/const/reg/apptype/apptype.py | 81 ++++++++++--------- pcapkit/corekit/enum.py | 18 +++-- pcapkit/corekit/fields/field.py | 6 +- pcapkit/corekit/fields/ipaddress.py | 12 +-- pcapkit/corekit/fields/numbers.py | 4 +- pcapkit/corekit/sentinels.py | 26 +++--- pcapkit/foundation/registry/protocols.py | 10 +-- pcapkit/protocols/application/http.py | 10 +-- pcapkit/protocols/internet/hip.py | 8 +- pcapkit/protocols/internet/hopopt.py | 4 +- pcapkit/protocols/internet/ipv6_opts.py | 4 +- pcapkit/protocols/internet/ipv6_route.py | 2 +- pcapkit/protocols/internet/mh.py | 56 ++++++------- pcapkit/protocols/link/ospf.py | 2 +- pcapkit/protocols/misc/pcap/frame.py | 2 +- pcapkit/protocols/misc/pcapng.py | 17 ++-- pcapkit/protocols/protocol.py | 4 +- pcapkit/protocols/schema/internet/hip.py | 9 ++- pcapkit/protocols/schema/internet/ipv4.py | 6 +- .../protocols/schema/internet/ipv6_route.py | 4 +- pcapkit/protocols/schema/internet/mh.py | 2 +- pcapkit/protocols/schema/misc/pcapng.py | 16 ++-- pcapkit/protocols/schema/schema.py | 4 +- pcapkit/protocols/schema/transport/tcp.py | 14 ++-- pcapkit/utilities/decorators.py | 2 +- pcapkit/utilities/exceptions.py | 2 +- pcapkit/vendor/__main__.py | 11 +-- pcapkit/vendor/ftp/command.py | 2 +- pcapkit/vendor/ipx/socket.py | 4 +- pcapkit/vendor/pcapng/tls_key_label.py | 4 +- pcapkit/vendor/reg/apptype/apptype.py | 81 ++++++++++--------- pcapkit/vendor/reg/ethertype.py | 2 +- pcapkit/vendor/reg/linktype.py | 2 +- 36 files changed, 236 insertions(+), 220 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 818b0d01ba..9bdf2528c6 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -51,7 +51,7 @@ name: "Lint" # have been deleted, not because the code changed. Already superseded the # same day it was measured, too: a cross-review measured `R0801 # duplicate-code` alone moving 400 -> 573 on `merge(f31e5114f, 83b58ebda)` -- -# this branch merged onto the `main` it was rebased against before #769 +# this branch merged onto the `main` it was rebased against before #768 # landed there -- taking R to 823 and the total to 6220 on that merge. Still # true of `main` itself today, since this PR touches no `pcapkit/` file, but # named for the tree it was actually measured on rather than asserted of @@ -62,7 +62,7 @@ name: "Lint" # actually unchanged between them, despite what an earlier revision of this # comment claimed: four files moved (`pcapkit/const/reg/apptype/apptype.py`, # `pcapkit/foundation/extraction.py`, `pcapkit/protocols/schema/schema.py`, -# `pcapkit/vendor/reg/apptype/apptype.py`; tree 7270ad50 -> 2b9ac808), #764's +# `pcapkit/vendor/reg/apptype/apptype.py`; tree 7270ad50 -> 2b9ac808), #758's # port-validation logic among them. E, W and C read identically either side of # that diff -- 90/4765/542 both times -- but 1a852698b is the tree actually # re-measured for this line, so that is what it is pinned to now, rather than diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index e84dc293ee..f49f456644 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -96,7 +96,7 @@ jobs: # `python_version < '3.12'` marker lets it install), installing it turned # 7 of the 10 HAS_PYPCAPFILE-gated methods this comment used to count # into failures, against a real bug in pcapkit/toolkit/pypcapfile.py's - # handling of pypcapfile 0.12.0's `IP.src`/`IP.dst`. #747 and #748 fixed + # handling of pypcapfile 0.12.0's `IP.src`/`IP.dst`. #743 and #746 fixed # that bug, and #751 gave the now-safe extra a home: the dedicated # `engine-tests` and `pypcap-parity` jobs below install it (15 # HAS_PYPCAPFILE-gated methods between them), deliberately apart from @@ -169,7 +169,7 @@ jobs: # PyPCAPFile is NOT added here either, even though this job's selection # also reaches test_new_engine_parity_runtime.py's 6 HAS_PYPCAPFILE # methods -- see the `test` job's comment above for the bug that used to - # make installing it here a regression, now fixed by #747/#748. #751's + # make installing it here a regression, now fixed by #743/#746. #751's # `pypcap-parity` job below covers those 6 methods instead, on the same # fixture-tier selection as this job but in a venv of its own. - name: Install package, test and generator dependencies @@ -251,7 +251,7 @@ jobs: # jobs is textually part of the one above it, and that guard's pytest_jobs() # requires exactly one such literal per job section -- a second one here, # even in a comment, makes it think this job has two install lines and - # cannot tell which extras the run would have. See #849's cross-review.), + # cannot tell which extras the run would have. See #845's cross-review.), # so "Engines Python 3.12/3.13/3.14" reported # green while never exercising PyPCAPFile at all on three of five legs -- # yet all three were promoted to ruleset 23497679's required checks anyway @@ -482,7 +482,7 @@ jobs: # comment here would make that guard think this section holds two # baseline install lines), and running them unchecked used to mean a # broken base install on a `not-installable` cell read as "expected" - # while the job had installed and run nothing at all (#849's + # while the job had installed and run nothing at all (#845's # cross-review). They now run under this step's own explicit `set -eu` # below -- not "the shell's default", since GitHub's default for a # Linux `run:` step is plain `bash -e {0}` with no `pipefail`, and @@ -546,7 +546,7 @@ jobs: # bare "5xx or 429" number scan: pip prints a "(NNN kB)" download # size on essentially every sdist fetch, and a Cython-generated # .c file runs to thousands of lines, so an unanchored bare-number - # match (#849's cross-review) fires on ordinary download-size + # match (#845's cross-review) fires on ordinary download-size # lines and on compiler diagnostics quoting a line number in that # range -- e.g. "Downloading pypcap-1.3.0.tar.gz (500 kB)" and # "pcap.c:501:24: error: ..." both matched, while the actual @@ -596,8 +596,9 @@ jobs: # those files themselves assert once they see this interpreter's # version, e.g. PyPCAPFile's own # test_unsupported_reason_tracks_the_running_interpreter and PyShark's - # test_the_reason_tracks_the_running_interpreter (added in #846) both - # branch on sys.version_info against the engine's own PYTHON_CEILING. + # test_the_reason_tracks_the_running_interpreter (added for PyShark's + # real-capture coverage) both branch on sys.version_info against the + # engine's own PYTHON_CEILING. # That is what proves the decline actually fired and named the # interpreter, rather than this job asserting it from the outside. # Skipped entirely for `not-installable` cells -- see the install step. @@ -815,7 +816,7 @@ jobs: # above, which each cover only the subset their own selection reaches. # PyPCAPFile is deliberately NOT added here -- see the `test` job's # comment above for the bug that used to make installing it a - # regression, now fixed by #747/#748. This job runs on 3.14 only, where + # regression, now fixed by #743/#746. This job runs on 3.14 only, where # PyPCAPFile's marker resolves to nothing regardless, so adding it here # would be a no-op that misleadingly suggests the extra is exercised by # `gate`; #751's `engine-tests` and `pypcap-parity` jobs cover it for @@ -986,7 +987,7 @@ jobs: # keeps that visibility, but means duplicating this job's system-package # install, per-engine install logic, and test invocation -- measured at # 182 raw lines across those three steps, and GitHub Actions has no YAML - # anchors to shrink that with -- into a second job immediately after #849 + # anchors to shrink that with -- into a second job immediately after #845 # rebuilt this one, to close a single unverified edge. Flagging the risk # here instead: if a `required-checks` run ever goes red with only a 3.15 # `engine-tests` leg failing underneath it, that is this exact gap and not diff --git a/pcapkit/const/ftp/command.py b/pcapkit/const/ftp/command.py index a8d538d200..7cd18554b0 100644 --- a/pcapkit/const/ftp/command.py +++ b/pcapkit/const/ftp/command.py @@ -35,7 +35,7 @@ class FEATCode(EnumRegistry, StrEnum): :meth:`~pcapkit.vendor.ftp.command.Command.process`) -- rather than minting the per-command ones at import time as an incidental side effect of building :class:`Command`'s own rows. GitHub issue #860: - that import-time mutation was the same defect shape #861 removed + that import-time mutation was the same defect shape #775 removed from :class:`~pcapkit.const.pcapng.filter_type.FilterType`, just not previously noticed here. ``_missing_`` still unmints for a keyword that turns up on the wire but names none of these -- the diff --git a/pcapkit/const/reg/apptype/apptype.py b/pcapkit/const/reg/apptype/apptype.py index 54bc17477c..80884bc76f 100644 --- a/pcapkit/const/reg/apptype/apptype.py +++ b/pcapkit/const/reg/apptype/apptype.py @@ -43,12 +43,12 @@ class TransportProtocol(EnumLookup, IntEnum): # ``undefined`` is declared explicitly as ``0`` and every other member # is ``auto()``, which continues from the preceding explicit value # rather than needing its own ``_start_ = 0`` to begin there; the - # owner's own ruling on this issue (#860) is explicit that + # a ruling given in review of the work for #860 is explicit that # ``undefined`` stays a direct ``0`` for that reason: "undefined # direct uses 0. then other real transport use auto. so we don't have # to define a _start_ and the undefined declaration is explicit." - # GitHub issue #808 already dropped the ``IntFlag`` base once - # nothing built a composite, and GitHub PR #836's ruling later + # GitHub issue #808 already dropped the ``IntFlag`` base once nothing + # built a composite, and a ruling given in review of that work later # retired ``|``-composite decoding entirely: "since it's no longer a # Flag, `|` joined values are no longer parsed and accepted, we will # treat it as a whole, instead of splitting." With no decoding left to @@ -61,7 +61,7 @@ class TransportProtocol(EnumLookup, IntEnum): # bare -- into anything narrower than the whole value it already is, on # either numbering; a plain ``dict.get`` lookup cannot tell a composed # ``int`` from any other one that happens to equal it. Treating every - # int as a whole is exactly what #836's ruling above asks for, and it + # int as a whole is exactly what that ruling above asks for, and it # is also why this renumbering changes what specific ints mean, not # only what composed ones do: # ``tcp | udp`` (``3``) used to name no registry and now resolves as @@ -108,22 +108,23 @@ def get(cls, key: 'int | str', default: 'Any' = NO_DEFAULT) -> 'TransportProtoco on these circumstances."* A stdlib ``E['nosuch']`` raises :exc:`KeyError`, and #923's census of the 127 concrete :class:`~pcapkit.corekit.enum.EnumLookup` subclasses -- taken before - #921 re-parented this class, so this class is not among them -- found - 119 already answering a name miss that way against 5 answering with - :exc:`ValueError`. Those 5 are :class:`AppType` and its four transport - registries, and they land there only because their own ``get()`` takes - an :class:`int` port and never accepts a name at all, rather than from - any name-miss policy. So there was no policy here to preserve, and a - name miss now reaches the caller as + the phase-2 re-parenting moved this class, so it is not among them -- + found 119 already answering a name miss that way against 5 answering + with :exc:`ValueError`. Those 5 are :class:`AppType` and its four + transport registries, and they land there only because their own + ``get()`` takes an :class:`int` port and never accepts a name at all, + rather than from any name-miss policy. So there was no policy here + to preserve, and a name miss now reaches the caller as :exc:`~pcapkit.utilities.exceptions.EnumKeyError` from the base. - Maintainer ruling on GitHub PR #836 -- "Do not allow extension of - TransportProtocol at all" -- is untouched by that: the refusal is still - a refusal and still mints nothing, only its exception class moved. + Maintainer ruling given in review of the work for #808 -- "Do not + allow extension of TransportProtocol at all" -- is untouched by + that: the refusal is still a refusal and still mints nothing, only + its exception class moved. The base is a :class:`classmethod` (:meth:`~pcapkit.corekit.enum.EnumLookup.get`), so this override moves from :class:`staticmethod` to :class:`classmethod` to - delegate at all -- the same move GitHub issue #908 and #915 made for + delegate at all -- the same move GitHub issue #908 made for :meth:`~pcapkit.const.http.method.Method.get`. Grepped every call site in this tree for GitHub issue #877: all call this method by name, none take it as a bare callable or introspect ``__func__``, @@ -163,19 +164,20 @@ def get(cls, key: 'int | str', default: 'Any' = NO_DEFAULT) -> 'TransportProtoco """ if isinstance(key, str): return super().get(key.lower(), default) - # NOTE: maintainer ruling on this PR (#836): "Do not allow extension - # of TransportProtocol at all." A name that is not a declared member - # used to mint a brand-new one here, at ``max_val + 1`` (before that, - # ``max_val * 2``) -- an unbounded, ever-growing set of transport - # protocols nothing ever asked for. There is nothing left to walk - # now: it is simply refused, exactly like any other unrecognised - # name -- including one spelling a composite, e.g. ``'tcp|udp'``. + # NOTE: maintainer ruling given in review of the work for #808: "Do + # not allow extension of TransportProtocol at all." A name that is + # not a declared member used to mint a brand-new one here, at + # ``max_val + 1`` (before that, ``max_val * 2``) -- an unbounded, + # ever-growing set of transport protocols nothing ever asked for. + # There is nothing left to walk now: it is simply refused, exactly + # like any other unrecognised name -- including one spelling a + # composite, e.g. ``'tcp|udp'``. # ``'|'`` used to be intercepted here on its own, so a composite in # disguise never got minted into a member whose own name lied about - # being a single transport; the owner's further ruling on this PR - # retired that special case along with the rest of the composite - # handling once TransportProtocol stopped being a Flag at all: - # "since it's no longer a Flag, `|` joined values are no longer + # being a single transport; a further ruling given in review of the + # work for #808 retired that special case along with the rest of the + # composite handling once TransportProtocol stopped being a Flag at + # all: "since it's no longer a Flag, `|` joined values are no longer # parsed and accepted, we will treat it as a whole, instead of # splitting." A ``'|'``-joined name is therefore not special any # more -- it is simply not the name of a declared member, and gets @@ -2524,10 +2526,10 @@ def _dispatch(cls, key: 'int', proto: 'TransportProtocol | str | int') -> 'Type[ the same as any caller passing a literal port-transport bitmask -- falls through to ``int.__or__`` and returns a bare :class:`int` rather than a member. Never split back into the - transports its bits would each name -- owner ruling on this PR - (#836) -- so it is looked up as the one whole value it - already is, exactly like any other bare int: refused when - that whole value names no registry, resolved when it + transports its bits would each name -- a ruling given in review + of the work for #808 -- so it is looked up as the one whole + value it already is, exactly like any other bare int: refused + when that whole value names no registry, resolved when it happens to equal one instead. Since GitHub issue #860 moved this class off power-of-two spacing, a hand-built composite is no longer guaranteed to be the former -- see @@ -2575,11 +2577,12 @@ def _dispatch(cls, key: 'int', proto: 'TransportProtocol | str | int') -> 'Type[ # either numbering. A genuine member reaching this point is # ``undefined`` -- the four real transports would already have # resolved above, and :meth:`TransportProtocol.get` cannot mint - # anything else, per this PR's own maintainer ruling against - # extending TransportProtocol at all -- and a bare :class:`int` - # reaches here whenever it matches no real member's value, e.g. a - # stray bit like ``17``. A composite built by hand used to reach - # here just as reliably, since no combination of the old power-of- + # anything else, per the maintainer ruling given in review of the + # work for #808, against extending TransportProtocol at all -- and + # a bare :class:`int` reaches here whenever it matches no real + # member's value, e.g. a stray bit like ``17``. A composite built by + # hand used to reach here just as reliably, since no combination of + # the old power-of- # two bits ever equalled a single real member's value; that is no # longer true under this class's current sequential numbering -- # ``TransportProtocol.tcp | TransportProtocol.udp`` (``3``) is @@ -2590,10 +2593,10 @@ def _dispatch(cls, key: 'int', proto: 'TransportProtocol | str | int') -> 'Type[ # and naming every transport whose bit was set -- the fix for # GitHub issue #759, where resolving a composite by picking its # lowest set bit dispatched every one containing ``tcp`` into the - # TCP registry regardless of what else it named. The owner's - # further ruling on this PR (#836) retired that decoding along with - # the rest of the composite handling: "since it's no longer a Flag, - # `|` joined values are no longer parsed and accepted, we will + # TCP registry regardless of what else it named. A further ruling + # given in review of the work for #808 retired that decoding along + # with the rest of the composite handling: "since it's no longer a + # Flag, `|` joined values are no longer parsed and accepted, we will # treat it as a whole, instead of splitting." So a composite's bits # are never decoded looking for a partial answer any more, on # either numbering -- it is looked up as the one whole value it diff --git a/pcapkit/corekit/enum.py b/pcapkit/corekit/enum.py index 7f4628ccbd..276e871375 100644 --- a/pcapkit/corekit/enum.py +++ b/pcapkit/corekit/enum.py @@ -47,8 +47,9 @@ the base above was deliberately behaviour-preserving on its own, so that it could land while other work was still in flight on the files the re-parent touches, and the phase itself landed in two pull requests for exactly that - reason -- #921 for the 17 enumerations that were free to move at once, and - `#930 `__ for the + reason -- the first for the 17 enumerations that were free to move at + once, and the second, `#930 + `__, for the remaining seven once the files holding them freed up. The registry tier's own shape is the earlier ruling on GitHub issue #842, @@ -340,12 +341,13 @@ def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self': on the same terms; the ``str`` path does not call the hook, for the reason given on :meth:`_validate_value` itself. - Both failure paths raise from :mod:`pcapkit.utilities.exceptions` rather - than a builtin, per the owner's ruling on GitHub issue #923: *"Either - ``ValueError`` or ``KeyError``, that's depending on how stdlib's - ``Enum`` would raise on these circumstances. And we should raise one - from ``pcapkit.utilities.exceptions`` rather builtin exceptions."* The - *shape* is unchanged by that ruling and deliberately so -- a name miss + Both failure paths raise from :mod:`pcapkit.utilities.exceptions` + rather than a builtin, per a ruling recorded on GitHub issue #923, + verbatim: *"Either ``ValueError`` or ``KeyError``, that's depending on + how stdlib's ``Enum`` would raise on these circumstances. And we + should raise one from ``pcapkit.utilities.exceptions`` rather builtin + exceptions."* The *shape* is unchanged by that ruling and + deliberately so -- a name miss stays :exc:`KeyError`-derived and a value miss :exc:`ValueError`-derived, matching ``E['nosuch']`` and ``E(999)`` on a stdlib :class:`~enum.Enum`, and matching the 119 of this tree's 127 concrete diff --git a/pcapkit/corekit/fields/field.py b/pcapkit/corekit/fields/field.py index a6caaf72f1..17ef3836e2 100644 --- a/pcapkit/corekit/fields/field.py +++ b/pcapkit/corekit/fields/field.py @@ -65,7 +65,7 @@ #: The sum therefore wants a budget, and this is the figure that makes one #: *safe*. A budget on its own is not: a capture cut short by its snapshot length #: pads legitimately and must keep parsing (#431, and the reasoning that declined -#: #571), and it pads far more than it reads, so any running budget tight enough +#: #554), and it pads far more than it reads, so any running budget tight enough #: to matter starts refusing real captures. Worse, it refuses them *sometimes* -- #: measured on this tree with a running budget alone, the same legitimate #: 54-octet frame parsed to one result on 37 of 40 calls and to another on calls @@ -207,7 +207,7 @@ def _zero_pad_budget() -> 'Iterator[list[int]]': #: sign after it (``'>Xs'``), or any other malformed template -- which is what #: makes checking for it a reliable way to tell those cases apart from each #: other *before* :func:`struct.calcsize` is asked to size either one. See -#: #825, #827. +#: #825. _RE_NEGATIVE_LENGTH_TEMPLATE = re.compile(r'^[@=<>!]?-\d+') @@ -542,7 +542,7 @@ def unpack(self, buffer: 'bytes | IO[bytes]', packet: 'dict[str, Any]') -> '_T': # what keeps the answer a function of *this* read rather than of # everything read before it. No shortfall a 16-bit wire length can produce # -- which is every shortfall a snapshot-truncated capture, a truncated - # option area or an over-long ``ihl`` can produce (#431, #571) -- is ever + # option area or an over-long ``ihl`` can produce (#431, #554) -- is ever # refused, on the first frame or the ten-thousandth. # # Nor may those small shortfalls *spend* the budget, which is why they are diff --git a/pcapkit/corekit/fields/ipaddress.py b/pcapkit/corekit/fields/ipaddress.py index 4260a1963e..ece4ef9357 100644 --- a/pcapkit/corekit/fields/ipaddress.py +++ b/pcapkit/corekit/fields/ipaddress.py @@ -90,12 +90,12 @@ def _reject_bool(value: 'object', description: str) -> 'None': warning**. On an IPv6-typed field the same conversion happens to raise instead, because the resulting :class:`~ipaddress.IPv4Address`'s version mismatches -- and that - asymmetry is exactly what let this slip past #481's otherwise + asymmetry is exactly what let this slip past #469's otherwise equivalent guard for :meth:`MH._make_opt_mn_id ` (c.f. #491). Every caller checks this *before* dispatching on the value's type, - for the same placement reason #481 gives: a correct check in the + for the same placement reason #469 gives: a correct check in the wrong position does not fire, and that placement mistake has already been made twice in this repository's history. @@ -147,7 +147,7 @@ def parse_ip_address(value: 'IPv4Address | IPv6Address | bytes | int | str', length is the only thing on the wire that carries the family -- must convert the argument itself, and that conversion happens **before** the schema, so it launders a :obj:`bool` into an - :class:`~ipaddress.IPv4Address` that #500's guard in + :class:`~ipaddress.IPv4Address` that the guard added for #491 in :meth:`_IPAddressField.pre_process` can then only see as a legitimate address. Seven such call sites took ``True`` / ``False`` without complaint as ``0.0.0.1`` / ``0.0.0.0`` -- or ``::1`` / ``::`` where the @@ -159,7 +159,7 @@ def parse_ip_address(value: 'IPv4Address | IPv6Address | bytes | int | str', unrelated defect of its own. Routing every one of them through here rather than giving each its own - :func:`isinstance` check is the whole point: #481 added exactly such a + :func:`isinstance` check is the whole point: #469 added exactly such a check to :meth:`MH._make_opt_mn_id `, and #491 was the same defect surviving at every site that had not been thought of. A @@ -167,14 +167,14 @@ def parse_ip_address(value: 'IPv4Address | IPv6Address | bytes | int | str', forgotten at the next one. The :obj:`bool` rejection is the **first** statement here, ahead of any - dispatch on the value's type, for the placement reason #481 gives and + dispatch on the value's type, for the placement reason #469 gives and :func:`_reject_bool` repeats. This raises :exc:`FieldValueError` and not :exc:`~pcapkit.utilities.exceptions.ProtocolError`, which is deliberate even though two sibling guards for the same mistake -- :meth:`MH._make_opt_mn_id - ` from #481 and + ` from #469 and :class:`ESP's SecurityAssociation ` from #491 -- raise the latter. The layer decides: this is a field-level conversion, so it diff --git a/pcapkit/corekit/fields/numbers.py b/pcapkit/corekit/fields/numbers.py index d2fdcde41f..893d3adc32 100644 --- a/pcapkit/corekit/fields/numbers.py +++ b/pcapkit/corekit/fields/numbers.py @@ -54,7 +54,7 @@ class NumberField(Field[int], Generic[_T]): ProtocolError: If ``bit_length`` is given negative. Left alone, ``(1 << bit_length) - 1`` raises a bare, uncatchable :exc:`ValueError` (``negative shift count``) here, before - :meth:`__call__`'s own negative-``length`` guard (#828/#829) or + :meth:`__call__`'s own negative-``length`` guard (#828) or :attr:`~pcapkit.corekit.fields.field.FieldBase.length`'s (#805) ever see anything -- this one fires at construction time, on the argument itself rather than on a resolved wire length. See @@ -153,7 +153,7 @@ def __call__(self, packet: 'dict[str, Any]') -> 'Self': a bare, uncatchable :exc:`ValueError` (``negative shift count``) when ``bit_length`` was not supplied, before :attr:`~pcapkit.corekit.fields.field.FieldBase.length` (see - its own :exc:`ProtocolError` guard, #805/#811/#827) or + its own :exc:`ProtocolError` guard, #805/#825) or :meth:`build_template` ever sees the value: this method sets ``self._bit_length`` from the resolved length eagerly, as a cache, and shifts by it immediately, so the crash happens on diff --git a/pcapkit/corekit/sentinels.py b/pcapkit/corekit/sentinels.py index 09daa7956d..8c9407b72e 100644 --- a/pcapkit/corekit/sentinels.py +++ b/pcapkit/corekit/sentinels.py @@ -238,22 +238,24 @@ class NoDefaultType: """Type of :data:`NO_DEFAULT`, the omitted-``default`` sentinel for :meth:`EnumLookup.get `. - A dedicated class rather than a bare :class:`object`, per the owner's ruling - on #859: *"use dedicated class rather than bare object. Follow the house - convention."* A bare :class:`object` compares under ``is`` exactly as - safely as a dedicated class with no ``__eq__`` of its own does -- identity - comparison was never the problem an earlier revision's docstring here - overstated it to be. What a bare :class:`object` actually lacks is a - readable representation: it prints as ```` in a - signature, in :func:`help`, and in a traceback, where ``NoDefaultType()`` + A dedicated class rather than a bare :class:`object`, per a ruling given in + review of the work for #857: *"use dedicated class rather than bare object. + Follow the house convention."* A bare :class:`object` compares under + ``is`` exactly as safely as a dedicated class with no ``__eq__`` of its + own does -- identity comparison was never the problem an earlier + revision's docstring here overstated it to be. What a bare + :class:`object` actually lacks is a readable representation: it prints + as ```` in a signature, in :func:`help`, and in + a traceback, where ``NoDefaultType()`` -- via :meth:`__repr__` below -- prints as ````. Named ``NoDefaultType`` for the *class* because that half of the house convention is settled: both :class:`NullType` and :class:`NoValueType` use ``Type``. At the time, the *instance*'s own name was not - similarly settled -- the owner's follow-up on #859 was explicit that - ``NULL`` (``SCREAMING_CASE``) and ``NoValue`` (``CapWords``) disagreed, and - "mainly depends on how we need it." The need here was continuity: + similarly settled -- a follow-up given in review of the work for #857 + was explicit that ``NULL`` (``SCREAMING_CASE``) and ``NoValue`` + (``CapWords``) disagreed, and "mainly depends on how we need it." The + need here was continuity: ``NO_DEFAULT`` was already the name on ``main`` -- referenced in :meth:`EnumLookup.get `'s signature, its docstring, and both comparison sites -- and that change was to *what @@ -358,7 +360,7 @@ class NoDefaultType: compares by value rather than identity, and reload staleness is a *tracked* defect class here for other constructs -- see :meth:`pcapkit.protocols.protocol.ProtocolBase._lookup_next_layer`'s own - docstring note citing GitHub issues #425, #428 and #560, and + docstring note citing GitHub issues #425 and #555, and :mod:`tests.protocols.test_dispatch_default_resolution_unit`'s own ``test_no_stale_class_survives_a_module_reload``, which reloads a module deliberately to pin the fix for exactly that class of bug elsewhere. A diff --git a/pcapkit/foundation/registry/protocols.py b/pcapkit/foundation/registry/protocols.py index 862582b04b..1106dc4c47 100644 --- a/pcapkit/foundation/registry/protocols.py +++ b/pcapkit/foundation/registry/protocols.py @@ -887,11 +887,11 @@ def register_apptype(code: 'int | Enum_AppType', module: 'str | ModuleDescriptor # up front, before any registry work, so every downstream use -- the # ``code.proto`` default and the registries lookup below -- sees members # only. Resolution is by the member's own ``name``, case-insensitively -- - # maintainer ruling on #815 -- via ``__members__`` directly rather than - # ``TransportProtocol[name]``: this function's contract is - # :exc:`~pcapkit.utilities.exceptions.RegistryError` for anything - # unrecognised, composite-spelled or not, and ``__getitem__`` raises a - # bare :exc:`KeyError` on a miss instead of that -- both + # a ruling given in review of the work for #806 -- via ``__members__`` + # directly rather than ``TransportProtocol[name]``: this function's + # contract is :exc:`~pcapkit.utilities.exceptions.RegistryError` for + # anything unrecognised, composite-spelled or not, and ``__getitem__`` + # raises a bare :exc:`KeyError` on a miss instead of that -- both # ``TransportProtocol['tcp|udp']`` and ``TransportProtocol['bogus']`` do, # now that GitHub issue #808 dropped the ``IntFlag`` base that used to # make the first of those two silently compose into the value ``3`` diff --git a/pcapkit/protocols/application/http.py b/pcapkit/protocols/application/http.py index 22b0e94783..a7d337838a 100644 --- a/pcapkit/protocols/application/http.py +++ b/pcapkit/protocols/application/http.py @@ -226,7 +226,7 @@ def _guess_version(self, length: 'int', **kwargs: 'Any') -> 'HTTP': preface came back ``version='2'`` only because ``httpv2.HTTP`` read its leading ``b'PRI'`` as a 24-bit declared frame length of 5,265,993, and ``b'foo bar baz\\r\\nX: y\\r\\n\\r\\n'`` -- not HTTP at all -- came back - ``version='2'`` the same way. #799/#802 closed the second of those by + ``version='2'`` the same way. #799 closed the second of those by requiring a frame's declared length to be backed by its buffer, but that left the preface *unidentifiable*: a real HTTP/2 connection opening is refused by both arms and reported as not-HTTP. @@ -313,7 +313,7 @@ def _guess_version(self, length: 'int', **kwargs: 'Any') -> 'HTTP': # version``, so a preface followed by a frame that used to trip the # #805 residual (an inner field shortfall, e.g. a 16-octet # ``GOAWAY``) had to reach the caller as something it could catch, - # not as a bare stdlib error. #811 has since closed that residual at + # not as a bare stdlib error. That residual has since been closed at # ``FieldBase.length``, so the same ``GOAWAY`` now raises # ``ProtocolError`` on its own and is caught by the ``except # ProtocolError: raise`` above, never reaching this clause -- but @@ -377,8 +377,8 @@ def _guess_version(self, length: 'int', **kwargs: 'Any') -> 'HTTP': # closed that particular route -- the same call now raises # ``ProtocolError: unknown HTTP version``, the documented answer. # Suppressing :exc:`struct.error` on the last arm stays regardless -- - # #811 kept it deliberately, as defence in depth, rather than retiring - # it now that the case it was added for is closed. Whether anything + # kept deliberately, as defence in depth, rather than retired now + # that the case it was added for is closed. Whether anything # can still reach it, and whether it should therefore go, is #825's # open question, not settled here. # @@ -396,7 +396,7 @@ def _guess_version(self, length: 'int', **kwargs: 'Any') -> 'HTTP': # field further in, past this guard's reach. Measured: a 16-octet # ``GOAWAY`` (``b'\x00\x00\x15\x07\x00\x00\x00\x00\x00' + b'\xff' * 7``) # used to raise a bare :exc:`struct.error` through ``httpv2.HTTP`` - # directly. #811 closed that class at its actual root -- + # directly. That class was closed at its actual root -- # :attr:`FieldBase.length # ` now catches # :func:`struct.calcsize`'s failure on a negative-length template and diff --git a/pcapkit/protocols/internet/hip.py b/pcapkit/protocols/internet/hip.py index bb1033c4e2..a0ff409f3e 100644 --- a/pcapkit/protocols/internet/hip.py +++ b/pcapkit/protocols/internet/hip.py @@ -259,10 +259,10 @@ class HIP(IPv6_Ext[Data_HIP, Schema_HIP], Internet[Data_HIP, Schema_HIP], schema=Schema_HIP, data=Data_HIP): """This class implements Host Identity Protocol. - Double-inherited, per the maintainer's convention on GitHub pull request - #924: a header that is *only* usable as an extension header inherits - :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` alone, while one - that is also usable as a standalone protocol names + Double-inherited, per the maintainer's convention given in review of the + work for #917: a header that is *only* usable as an extension header + inherits :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` alone, + while one that is also usable as a standalone protocol names :class:`~pcapkit.protocols.internet.internet.Internet` as well. HIP is both, on two independent grounds: diff --git a/pcapkit/protocols/internet/hopopt.py b/pcapkit/protocols/internet/hopopt.py index 973075e01e..6f9b5a2ea2 100644 --- a/pcapkit/protocols/internet/hopopt.py +++ b/pcapkit/protocols/internet/hopopt.py @@ -478,12 +478,12 @@ def _hopopt_option_length(schema_len: 'int') -> 'int': excludes the Option Type and Opt Data Len fields themselves, so the whole option, which is what every ``_read_opt_*`` below reports back as the parsed option's own ``.length``, is two octets more. This is - the exact ``+2``/``-2`` mismatch #398 fixed independently in six + the exact ``+2``/``-2`` mismatch independently fixed in six places (see ``Data_PadOption.length`` vs. ``Schema_PadOption.length`` below, at the surviving explanation of that fix); collecting the read-side half of it into one helper is so a future fix to this arithmetic only has to happen once. Do NOT drop the ``+ 2``: that is - precisely the mismatch #398 fixed. + precisely this mismatch. Section 4.2 is the citation because it is what defines the TLV option format, and it is where the sentence quoted above actually appears. diff --git a/pcapkit/protocols/internet/ipv6_opts.py b/pcapkit/protocols/internet/ipv6_opts.py index b8a0b65a1c..17e8e291bb 100644 --- a/pcapkit/protocols/internet/ipv6_opts.py +++ b/pcapkit/protocols/internet/ipv6_opts.py @@ -470,12 +470,12 @@ def _ipv6_opts_option_length(schema_len: 'int') -> 'int': excludes the Option Type and Opt Data Len fields themselves, so the whole option, which is what every ``_read_opt_*`` below reports back as the parsed option's own ``.length``, is two octets more. This is - the exact ``+2``/``-2`` mismatch #398 fixed independently in six + the exact ``+2``/``-2`` mismatch independently fixed in six places (see ``Data_PadOption.length`` vs. ``Schema_PadOption.length`` below, at the surviving explanation of that fix); collecting the read-side half of it into one helper is so a future fix to this arithmetic only has to happen once. Do NOT drop the ``+ 2``: that is - precisely the mismatch #398 fixed. + precisely this mismatch. Section 4.2 is the citation because it is what defines the TLV option format, and it is where the sentence quoted above actually appears. diff --git a/pcapkit/protocols/internet/ipv6_route.py b/pcapkit/protocols/internet/ipv6_route.py index 924165a253..fb41e7b8dc 100644 --- a/pcapkit/protocols/internet/ipv6_route.py +++ b/pcapkit/protocols/internet/ipv6_route.py @@ -611,7 +611,7 @@ def _read_data_type_rpl(self, schema: 'Schema_RPL', *, header: 'Schema_IPv6_Rout # ``header.length`` is ``Hdr Ext Len``, in the 8-octet units # :rfc:`6554#section-3` specifies, not octets -- and it additionally # assumed 16-octet addresses, which an SRH only carries when ``CmprI`` - # and ``CmprE`` are both 0. #489 called it out but deliberately left + # and ``CmprE`` are both 0. #487 called it out but deliberately left # it alone, because ``RPL.post_process`` raised on every pack back # then so there was no round trip to validate a replacement against. # #556 removed that blocker and #564 the mis-sized fixed area behind diff --git a/pcapkit/protocols/internet/mh.py b/pcapkit/protocols/internet/mh.py index 729400e1f4..23651ebfbc 100644 --- a/pcapkit/protocols/internet/mh.py +++ b/pcapkit/protocols/internet/mh.py @@ -581,11 +581,11 @@ class FastBindingAcknowledgmentStatus(EnumLookup, IntEnum): This class carried its own hand-rolled ``get()`` override through #930, and briefly again through GitHub issue #935's first attempt, which widened the override to accept ``default`` rather than delete - it outright. The owner's final ruling on #935 went the other way, - verbatim -- asked *"why must we have the two overrides tho? cant - they directly fall back to the base class's?"*, the answer was *"I - prefer (2) directly"*, ``(2)`` naming deletion among the ruling's - own options. Measured before acting on it: the override's own + it outright. A ruling given in review of the work for #935 went the + other way, verbatim -- asked *"why must we have the two overrides + tho? cant they directly fall back to the base class's?"*, the answer + was *"I prefer (2) directly"*, ``(2)`` naming deletion among the + ruling's own options. Measured before acting on it: the override's own docstring called it a "Backport support for original codes", but this class mints no alias -- ``__members__`` and ``list(cls)`` agree at 6 -- so what the override actually did was resolve an @@ -698,11 +698,11 @@ class IPv6AddressPrefixCode(EnumLookup, IntEnum): This class carried its own hand-rolled ``get()`` override through #930, and briefly again through GitHub issue #935's first attempt, which widened the override to accept ``default`` rather than delete - it outright. The owner's final ruling on #935 went the other way, - verbatim -- asked *"why must we have the two overrides tho? cant - they directly fall back to the base class's?"*, the answer was *"I - prefer (2) directly"*, ``(2)`` naming deletion among the ruling's - own options. Measured before acting on it: the override's own + it outright. A ruling given in review of the work for #935 went the + other way, verbatim -- asked *"why must we have the two overrides + tho? cant they directly fall back to the base class's?"*, the answer + was *"I prefer (2) directly"*, ``(2)`` naming deletion among the + ruling's own options. Measured before acting on it: the override's own docstring called it a "Backport support for original codes", but this class mints no alias -- ``__members__`` and ``list(cls)`` agree at 4 -- so what the override actually did was resolve an @@ -838,9 +838,10 @@ class LocalizedRoutingStatus(EnumLookup, IntEnum): :class:`IPv6AddressPrefixCode` either: it had zero callers repo-wide -- tests included -- so GitHub issue #880 deleted it outright rather than rebuilding it on the immutable contract, the same conclusion - #935 reached separately for the other two, on the owner's ruling - there, verbatim: *"I prefer (2) directly"* -- ``(2)`` being deletion - of those two overrides rather than widening them to match the base. + #935 reached separately for the other two, on a ruling given in + review of that work, verbatim: *"I prefer (2) directly"* -- ``(2)`` + being deletion of those two overrides rather than widening them to + match the base. GitHub issue #930's re-parenting above gives this class ``get``/``get_all`` again, but as the base's own bare lookup rather than a bespoke override -- it still cannot mint, so an unassigned @@ -917,9 +918,10 @@ class LMAAddressCode(EnumLookup, IntEnum): :class:`IPv6AddressPrefixCode` either: it had zero callers repo-wide -- tests included -- so GitHub issue #880 deleted it outright rather than rebuilding it on the immutable contract, the same conclusion - #935 reached separately for the other two, on the owner's ruling - there, verbatim: *"I prefer (2) directly"* -- ``(2)`` being deletion - of those two overrides rather than widening them to match the base. + #935 reached separately for the other two, on a ruling given in + review of that work, verbatim: *"I prefer (2) directly"* -- ``(2)`` + being deletion of those two overrides rather than widening them to + match the base. GitHub issue #930's re-parenting above gives this class ``get``/``get_all`` again, but as the base's own bare lookup rather than a bespoke override -- it still cannot mint, so an unassigned @@ -2903,13 +2905,13 @@ def _mh_option_length(schema_length: 'int') -> 'int': Length fields"* -- so the whole option, which is what every ``_read_opt_*`` below reports back as the parsed option's own ``.length``, is two octets more. This is the exact ``+2``/``-2`` - mismatch #398 fixed independently in six places (see the ``Note:`` + mismatch independently fixed in six places (see the ``Note:`` on :meth:`_read_opt_pad` below, which explains why a ``Pad1`` option -- the one option with no ``Option Length`` field at all -- is this helper's sole exception); collecting the read-side half of it into one helper is so a future fix to this arithmetic only has - to happen once. Do NOT drop the ``+ 2``: that is precisely the - mismatch #398 fixed. + to happen once. Do NOT drop the ``+ 2``: that is precisely this + mismatch. The ``+ 2`` is specific to an :rfc:`6275#section-6.2` mobility option, whose Option Type and Option Length are one octet each. It @@ -7972,7 +7974,7 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption (RFC 4283's ``user@realm`` form) rather than a numeric identifier, so there is no non-arbitrary int-to-text mapping the way there is int-to-address or int-to-octets, and an - :obj:`int` is rejected there (c.f. #467, #468). + :obj:`int` is rejected there (c.f. #467). **kwargs: Arbitrary keyword arguments. Returns: @@ -7994,7 +7996,7 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption type its subtype's field cannot hold at all: anything but :obj:`str` for ``NAI``, anything but :obj:`bytes`/ :obj:`bytearray`/:obj:`int` for the other six -- an :obj:`int` - is converted rather than rejected there, per #468 -- or anything + is converted rather than rejected there, per #467 -- or anything :class:`ipaddress.IPv6Address` itself does not accept for ``IPv6_Address`` (c.f. #469). @@ -8015,14 +8017,14 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption # exactly the class of leak this handler exists to stop, and which a # guard living inside the ``elif isinstance(identifier, int)`` branch # could not catch, since the ``IPv6_Address`` dispatch never reaches - # it (c.f. #467, #468). + # it (c.f. #467). try: # ``Enum_MNIDSubtype(subtype_val)`` round-trips a plain int back # into a named member for the message below -- but its own # ``_missing_`` only auto-extends 9-15 and 16-255, so 0, # negatives and anything above 255 make the constructor itself # raise a bare ``ValueError``, which would defeat the point of - # this guard (c.f. #468 review). Caught here and the raw value + # this guard (c.f. #467). Caught here and the raw value # used instead rather than let it propagate. subtype_repr = repr(Enum_MNIDSubtype(subtype_val)) except ValueError: @@ -8063,7 +8065,7 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption # unguarded, :class:`ipaddress.IPv6Address` raises # ``AddressValueError``, itself a bare :exc:`ValueError`, so this # handler would otherwise ship with its lower bound guarded and its - # upper bound leaking (c.f. #467, #468). Checked explicitly rather + # upper bound leaking (c.f. #467). Checked explicitly rather # than by wrapping the construction below, because that would also # swallow the wrong-*type* ``AddressValueError`` -- a ``str`` or # ``None`` reaching here -- which is #469's subject, not this one's. @@ -8080,7 +8082,7 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption # bytes, int or str). The ``try`` wraps only this call, not # the whole branch, so it cannot swallow the ProtocolError # raised above for an out-of-range int, which is also a - # ValueError subclass (c.f. #467, #468, #469). + # ValueError subclass (c.f. #467, #469). try: identifier = ipaddress.IPv6Address(identifier) except ValueError as error: @@ -8114,9 +8116,9 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption # width all along -- the pre-#467 defect was never the sizing, it # was that ``identifier`` itself stayed an ``int`` afterwards and # was handed to ``BytesField`` unconverted, which ``struct.pack()`` - # cannot do anything with. #468 initially rejected outright instead + # cannot do anything with. #467 initially rejected outright instead # of noticing that; converting is what this revision does (c.f. - # #467, #468). ``bit_length()`` is 0 for 0 itself, which would + # #467). ``bit_length()`` is 0 for 0 itself, which would # otherwise declare a zero-octet identifier -- collapsing "the # identifier's value is 0" into "there is no identifier" -- so the # width is floored at one octet, matching what any reasonable diff --git a/pcapkit/protocols/link/ospf.py b/pcapkit/protocols/link/ospf.py index 9438cad27b..695957a31f 100644 --- a/pcapkit/protocols/link/ospf.py +++ b/pcapkit/protocols/link/ospf.py @@ -336,7 +336,7 @@ def _make_id_numbers(self, id: 'IPv4Address | str | bytes | bytearray') -> 'byte straight from its own arguments), so the only caller is a unit test. It is routed through :func:`parse_ip_address` anyway, so that it does not resurface the defect the moment a caller - reaches it -- the same kind of omission is how #481's single-site + reaches it -- the same kind of omission is how #469's single-site fix survived to become #491 and then #508 (c.f. #540). The description below uses :attr:`self.__class__.__name__ diff --git a/pcapkit/protocols/misc/pcap/frame.py b/pcapkit/protocols/misc/pcap/frame.py index e28de84ff4..a391b386cc 100644 --- a/pcapkit/protocols/misc/pcap/frame.py +++ b/pcapkit/protocols/misc/pcap/frame.py @@ -278,7 +278,7 @@ def read(self, length: 'Optional[int]' = None, *, _read: 'bool' = True, # other way round, which is what this reader used to do (see #618). # # The two only differ for a frame the snapshot length cut short, so - # until ``big_endian.pcap`` arrived with #614 no fixture here could + # until ``big_endian.pcap`` arrived with #605 no fixture here could # tell the two assignments apart. # # Worth knowing *why* this reader moved rather than the other one, diff --git a/pcapkit/protocols/misc/pcapng.py b/pcapkit/protocols/misc/pcapng.py index 9abcc97ee5..1f4033c540 100644 --- a/pcapkit/protocols/misc/pcapng.py +++ b/pcapkit/protocols/misc/pcapng.py @@ -293,14 +293,15 @@ class PacketReception(EnumLookup, enum.IntEnum): # :class:`pcapkit.const.pcapng.tls_key_label.TLSKeyLabel`, generated the same # way as its :mod:`pcapkit.const.pcapng` siblings -- imported above as # ``Enum_TLSKeyLabel``, matching the ``Enum_*`` alias every one of its seven -# :mod:`pcapkit.const.pcapng` siblings already carries in this file (the -# owner's review on #890: this import was the only one of the eight lacking -# it, because the class used to be *defined* here rather than imported, so -# nothing applied the convention until this move made it an import). The -# assignment below re-exports the same object under the module's own, -# unaliased name, so ``from pcapkit.protocols.misc.pcapng import -# TLSKeyLabel`` keeps working and still resolves to the identical class -- -# not a copy -- that every internal ``Enum_TLSKeyLabel`` reference below uses. +# :mod:`pcapkit.const.pcapng` siblings already carries in this file (a +# review of the work for #886: this import was the only one of the +# eight lacking it, because the class used to be *defined* here rather than +# imported, so nothing applied the convention until this move made it an +# import). The assignment below re-exports the same object under the +# module's own, unaliased name, so ``from pcapkit.protocols.misc.pcapng +# import TLSKeyLabel`` keeps working and still resolves to the identical +# class -- not a copy -- that every internal ``Enum_TLSKeyLabel`` reference +# below uses. # :class:`WireGuardKeyLabel` below stays hand-written -- it is verified closed # (draft-ietf-opsawg-pcapng-06 section 4.7's "is one of" four names) and not a # registry, so filing it under :mod:`pcapkit.const` would misrepresent it as diff --git a/pcapkit/protocols/protocol.py b/pcapkit/protocols/protocol.py index 2a5c5bacdc..7ae245ccea 100644 --- a/pcapkit/protocols/protocol.py +++ b/pcapkit/protocols/protocol.py @@ -1741,8 +1741,8 @@ def _lookup_next_layer(registry: 'DefaultDict[int, ModuleDescriptor[ProtocolBase :func:`importlib.import_module` -- see #574. Memoising the resolved class here instead, whether under ``proto``, in ``registry``'s default factory, or in a cache beside the registry, would retain a - class that :func:`importlib.reload` then makes stale; #425 and #428 - at this layer and #560 at the schema layer are all that same defect. + class that :func:`importlib.reload` then makes stale; #425 at + this layer and #555 at the schema layer are all that same defect. """ protocol = ProtocolBase._lookup_registry(registry, proto) diff --git a/pcapkit/protocols/schema/internet/hip.py b/pcapkit/protocols/schema/internet/hip.py index 1cbf147893..406a0f72d8 100644 --- a/pcapkit/protocols/schema/internet/hip.py +++ b/pcapkit/protocols/schema/internet/hip.py @@ -572,7 +572,7 @@ class R1CounterParameter(Parameter, code=[Enum_Parameter.R1_Counter, layout to code 128 (``R1_Counter``) and code 129 (``R1_COUNTER``) -- one parameter under two numbers, the difference being HIP's own C-bit rather than an unrelated code -- and the field list below is that layout - exactly, since #696 widened :attr:`counter` to eight octets. + exactly, since #672 widened :attr:`counter` to eight octets. :attr:`~pcapkit.protocols.internet.hip.HIP.__parameter__` already carries two hand-written entries -- not a name-normalisation rule; ``R1_Counter`` and ``R1_COUNTER`` differ only in case, and each needed its own line -- @@ -681,9 +681,10 @@ class LocatorSetParameter(Parameter, code=Enum_Parameter.LOCATOR_SET): #: parameter's packet context while they pack, and their own ``len`` #: overwrites it before ``padding`` is reached. #: - #: This is the site #651 deliberately left alone and #664 documented as an - #: exclusion, because two defects in this parameter cancelled at the shape - #: its tests sampled and correcting either alone made the wire output worse. + #: This is the site #651 deliberately left alone, and its fix + #: documented as an exclusion, because two defects in this parameter + #: cancelled at the shape its tests sampled and correcting either alone + #: made the wire output worse. #: The shadowed ``len`` is 4 for any IPv6 locator, so the old expression #: appended exactly four octets whatever the locator count; and the wrong #: ``Length`` unit above made the declared ``Length`` ``4n`` where the diff --git a/pcapkit/protocols/schema/internet/ipv4.py b/pcapkit/protocols/schema/internet/ipv4.py index ca9b9b1767..393db80bb7 100644 --- a/pcapkit/protocols/schema/internet/ipv4.py +++ b/pcapkit/protocols/schema/internet/ipv4.py @@ -368,8 +368,8 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Schema': of, so nothing downstream can question it. Measured before this fix: ``ts_data=[True, 5]`` packed as ``0000000100000005`` and reported ``IPv4Address('0.0.0.1')`` with no exception and no - warning. This was the fifth site of that defect -- #481, #500, - #539 and #540 are the first four -- and the reason it is the fifth + warning. This was the fifth site of that defect -- #469, #491, + #508 and #540 are the first four -- and the reason it is the fifth is that each of those fixed the sites it could see. See #552. """ @@ -434,7 +434,7 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Schema': # still raises a plain :exc:`ValueError` for a tail that is not a # whole number of 8-octet pairs, which no ``except BaseError`` can # catch, and because leaving one of this method's three conversions - # unguarded is exactly how #552 came to be the fifth site of #481. + # unguarded is exactly how #552 came to be the fifth site of #469. pad = self.remainder for index in range(0, len(pad), 8): buf_ip = pad[index:index + 4] diff --git a/pcapkit/protocols/schema/internet/ipv6_route.py b/pcapkit/protocols/schema/internet/ipv6_route.py index f480cebb1c..83ac62b7fa 100644 --- a/pcapkit/protocols/schema/internet/ipv6_route.py +++ b/pcapkit/protocols/schema/internet/ipv6_route.py @@ -81,10 +81,10 @@ def ipv6_route_header_length(hdr_ext_len: 'int') -> 'int': :mod:`pcapkit.protocols.internet.ipv6_route` reports back as the parsed route data's own ``.length``, which :meth:`~pcapkit.protocols.internet. ipv6_route.IPv6_Route.read` then subtracts from the outer packet length - to find the next layer's length. #489 unified the write side + to find the next layer's length. #487 unified the write side (:meth:`~pcapkit.protocols.internet.ipv6_route.IPv6_Route._make_hdr_ext_len`) into one helper; this is the matching read-side helper for the total - header length, finishing that half of #487/#489. + header length, finishing that half of #487. Args: hdr_ext_len: raw ``Hdr Ext Len`` field value, as read off the wire. diff --git a/pcapkit/protocols/schema/internet/mh.py b/pcapkit/protocols/schema/internet/mh.py index 7b475ecfc3..995b8c534e 100644 --- a/pcapkit/protocols/schema/internet/mh.py +++ b/pcapkit/protocols/schema/internet/mh.py @@ -741,7 +741,7 @@ class MNIDOption(Option, code=Enum_Option.MN_ID_OPTION_TYPE): # :class:`~pcapkit.corekit.fields.strings.StringField` or # :class:`~pcapkit.corekit.fields.strings.BytesField`, and handing either # a raw ``int`` is precisely the #467 defect -- ``struct.pack()`` cannot - # consume it (c.f. #467, #468). + # consume it (c.f. #467). def __init__(self, type: 'Enum_Option', length: 'int', subtype: 'Enum_MNIDSubtype', identifier: 'bytes | str | IPv6Address') -> 'None': ... diff --git a/pcapkit/protocols/schema/misc/pcapng.py b/pcapkit/protocols/schema/misc/pcapng.py index 4cbe91b294..a40d1d1937 100644 --- a/pcapkit/protocols/schema/misc/pcapng.py +++ b/pcapkit/protocols/schema/misc/pcapng.py @@ -316,12 +316,12 @@ def bounded_option(length: 'Callable[[dict[str, Any]], int]') -> 'Callable[[dict with one option declaring 65,535 against none present, produced 131,070,000 octets of zero padding, an amplification of 1,637x linear in the block count. See `#594 `__, - and `#593 `__ for the + and `#573 `__ for the 32-bit band the field layer's own budget already covers. The bound has to come from this layer because the field layer cannot see it. What distinguishes the crafted case from the legitimate one is not the - shortfall's size -- both are inside a 16-bit length, which is why #571's + shortfall's size -- both are inside a 16-bit length, which is why #554's ``len(buffer) < length`` rejection was declined -- but whether the option is inconsistent with the framing the block itself declares. Block Total Length is authoritative and cross-checked against its own trailing copy, so @@ -804,7 +804,7 @@ def register(code: 'Enum_OptionType', cls: 'Type[Option]', ns: 'Optional[str]' = every other registry in the package does -- the lookup that follows cannot tell a deliberate replacement from an accidental one, so an unreported overwrite is a parser silently swapped out for another. See - `#681 `__ for the + `#675 `__ for the guard ``register_protocol`` added first, which this one now matches. The guard is identity-based: it fires only when the incumbent differs @@ -1865,8 +1865,8 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Self': names them, and a blank line -- found by *reading*, not by splitting -- is what starts the next entry. The one-octet terminator that must follow a binary field's value, and the warning - when it is missing, are unchanged from `#722 - `__; walking the + when it is missing, are unchanged from `#704 + `__; walking the buffer whole rather than pre-slicing it also retires that fix's newline restoration, which existed only to undo what the slicing itself had taken away. @@ -2016,10 +2016,10 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Self': # the reader's position is well defined either way -- # exactly length + 1 octets past where the field name # started -- so a bad octet here ends only this - # entry's field collection, matching #722: it does not + # entry's field collection, matching #704: it does not # abort the walk, which keeps looking for the next - # entry's separator from here. See #728's review for - # why an outer abort was considered and rejected as + # entry's separator from here. See #723 for why an + # outer abort was considered and rejected as # the default. terminator = entry_data.read(1) if terminator != b'\n': diff --git a/pcapkit/protocols/schema/schema.py b/pcapkit/protocols/schema/schema.py index ddb595bc69..e2e79640bb 100644 --- a/pcapkit/protocols/schema/schema.py +++ b/pcapkit/protocols/schema/schema.py @@ -1065,7 +1065,7 @@ class _EnumRegistry(collections.defaultdict): This is the schema-layer instance of the defect :meth:`ProtocolBase.\ _lookup_registry ` fixed for the protocol-layer ``__proto__`` family in GitHub issues #421 and - #425/#428; see GitHub issue #555. The fallback itself is deliberate -- it + #425; see GitHub issue #555. The fallback itself is deliberate -- it is how an unknown option, chunk or block falls back to its ``Unknown*``/``Unassigned*`` schema -- so this subclass keeps returning it, it just stops recording it. @@ -1305,7 +1305,7 @@ def register(cls, code: '_ET', schema: 'Type[Self]') -> 'None': have made the next legitimate registration for that code warn about an entry no caller ever asked for -- the defect fixed for this layer in #555, and for the parser-layer ``__proto__`` family in #421 and - #425/#428. That fix is what makes this guard safe to add. + #425. That fix is what makes this guard safe to add. :class:`pcapkit.protocols.schema.misc.pcapng.Option` overrides this method with a namespaced registry of its own and does not delegate diff --git a/pcapkit/protocols/schema/transport/tcp.py b/pcapkit/protocols/schema/transport/tcp.py index 07fbef5564..b3272436d9 100644 --- a/pcapkit/protocols/schema/transport/tcp.py +++ b/pcapkit/protocols/schema/transport/tcp.py @@ -276,7 +276,7 @@ def mptcp_dss_ack_selector(pkt: 'dict[str, Any]') -> 'Field': integer``. Measured on the 8-octet form, which the old lambda did reach: ``_make_mptcp_dss(DSS, ack=1 << 40)`` raised exactly that. - That half is now history: **#598 fixed it**, in + That half is now history: **#591 fixed it**, in :mod:`pcapkit.corekit.fields.numbers` where this note used to say the fix belonged, by recomputing ``_need_process`` from the width actually in force instead of once from the placeholder. A callable-length @@ -334,8 +334,8 @@ def mptcp_dss_dsn_selector(pkt: 'dict[str, Any]') -> 'Field': identical defect: ``NumberField(length=lambda pkt: 8 if pkt['flags']['m'] else 0, ...)``. See that function's note for why the ``0`` was wrong, why a corrected lambda would not have packed either *at the time*, and why the - ``SwitchField`` form is kept now that #598 has made a callable length work. - C.f. #576, #598. + ``SwitchField`` form is kept now that #591 has made a callable length work. + C.f. #576, #591. """ if not pkt['flags']['M']: @@ -409,12 +409,12 @@ def post_process(self, value: 'int | bytes', packet: 'dict[str, Any]') -> 'Enum_ A port outside this field's own width is rejected *before* any of that, rather than being let through to :meth:`_missing_` and caught alongside a genuine miss. Both are a bare :exc:`ValueError` - with nothing to tell them apart by type, and GitHub issue #764 + with nothing to tell them apart by type, and GitHub issue #758 gave the out-of-range case a deliberate, ``breaking``-tagged rejection specifically so it would stop being minted over -- a catch keyed on exception type alone cannot see the difference between that and :mod:`aenum`'s own "no member has this value", - so it would absorb both and quietly revert #764 for these four + so it would absorb both and quietly revert #758 for these four fields. Checking the width first needs no exception-based distinction at all: it asks the same question :meth:`_missing_` would eventually ask, and asks it in a way that never manufactures @@ -432,7 +432,7 @@ def post_process(self, value: 'int | bytes', packet: 'dict[str, Any]') -> 'Enum_ value = super(EnumField, self).post_process(value, packet) proto = Enum_TransportProtocol.tcp if not (isinstance(value, int) and 0 <= value < (1 << (8 * self.length))): - # NOTE: lets AppType.get() -- unmodified -- raise #764's rejection + # NOTE: lets AppType.get() -- unmodified -- raise #758's rejection # for a port this field's own width cannot represent, rather than # risking it being absorbed below as a foreign miss. return self._namespace.get(value, proto=proto) @@ -444,7 +444,7 @@ def post_process(self, value: 'int | bytes', packet: 'dict[str, Any]') -> 'Enum_ # NOTE: value is already known to be in-width here, so this # ValueError is aenum's own "no member has this value" for an # in-range but unassigned port -- a foreign miss, absorbed -- - # never #764's out-of-range rejection, which never reaches + # never #758's out-of-range rejection, which never reaches # this branch. A pcapkit.utilities.exceptions error is still a # deliberate registry decision and propagates unchanged. if isinstance(error, BaseError): diff --git a/pcapkit/utilities/decorators.py b/pcapkit/utilities/decorators.py index 1b59496dec..19999cb785 100644 --- a/pcapkit/utilities/decorators.py +++ b/pcapkit/utilities/decorators.py @@ -172,7 +172,7 @@ def behold(*args: 'P.args', **kwargs: 'P.kwargs') -> 'R_beholder': # whether or not it is registered. So both *unknown* paths preserve # the code that arrived; what differs is only that SCTP's is an # enumeration the protochain can name, while TCP's is an ``int`` and - # renders as ``Raw``. That was #418, closed by #426; this line is + # renders as ``Raw``. That was #418; this line is # about the *failure* path, which it makes uniform. next_ = protocol(file_, length, error=str(exc), alias=proto) return cast('R_beholder', next_) diff --git a/pcapkit/utilities/exceptions.py b/pcapkit/utilities/exceptions.py index 7c55b35bdf..47417cc16c 100644 --- a/pcapkit/utilities/exceptions.py +++ b/pcapkit/utilities/exceptions.py @@ -485,7 +485,7 @@ class EnumKeyError(BaseError, KeyError): taste: ``E['nosuch']`` raises :exc:`KeyError` and ``E(999)`` raises :exc:`ValueError`, so a lookup that misses by *name* is :exc:`KeyError`-derived and one that misses by *value* is - :exc:`ValueError`-derived. The owner's ruling on GitHub issue #923, + :exc:`ValueError`-derived. A ruling recorded on GitHub issue #923, verbatim: *"Either ``ValueError`` or ``KeyError``, that's depending on how stdlib's ``Enum`` would raise on these circumstances. And we should raise one from ``pcapkit.utilities.exceptions`` rather builtin exceptions."* diff --git a/pcapkit/vendor/__main__.py b/pcapkit/vendor/__main__.py index 814b985ff3..92eb770e4f 100644 --- a/pcapkit/vendor/__main__.py +++ b/pcapkit/vendor/__main__.py @@ -51,11 +51,12 @@ def get_parser() -> 'ArgumentParser': def _snapshot_and_restore(vendor: 'Type[Vendor]') -> 'Iterator[None]': """Copy a target's const file aside before it runs; restore it if it raises. - The owner's ruling on GitHub issue #872, verbatim: *"an easier path is - simply keep a copy before running the sub-vendor and revert if anything - failed."* This is that -- at the per-target boundary :func:`run` already - owns, which is also exactly where the ruling's ``(b)``, "only discard - changes made by a non-zero sub-vendor", wants the discarding to happen. + A ruling given in review of the work for #872, verbatim: *"an easier + path is simply keep a copy before running the sub-vendor and revert if + anything failed."* This is that -- at the per-target boundary + :func:`run` already owns, which is also exactly where the ruling's + ``(b)``, "only discard changes made by a non-zero sub-vendor", wants + the discarding to happen. It is a *wider* guarantee than protecting the single ``open``/``print`` pair :meth:`~pcapkit.vendor.default.Vendor.__init__` diff --git a/pcapkit/vendor/ftp/command.py b/pcapkit/vendor/ftp/command.py index e97c9d7abc..5ec3ff1091 100644 --- a/pcapkit/vendor/ftp/command.py +++ b/pcapkit/vendor/ftp/command.py @@ -75,7 +75,7 @@ class FEATCode(EnumRegistry, StrEnum): :meth:`~pcapkit.vendor.ftp.command.Command.process`) -- rather than minting the per-command ones at import time as an incidental side effect of building :class:`Command`'s own rows. GitHub issue #860: - that import-time mutation was the same defect shape #861 removed + that import-time mutation was the same defect shape #775 removed from :class:`~pcapkit.const.pcapng.filter_type.FilterType`, just not previously noticed here. ``_missing_`` still unmints for a keyword that turns up on the wire but names none of these -- the diff --git a/pcapkit/vendor/ipx/socket.py b/pcapkit/vendor/ipx/socket.py index 8809f93a14..b470148f2a 100644 --- a/pcapkit/vendor/ipx/socket.py +++ b/pcapkit/vendor/ipx/socket.py @@ -75,7 +75,7 @@ # NOTE: 0x0000 was never listed as a well-known socket in the scraped # registry table, but it is the IPX protocol's own default for the # ``dst``/``src`` socket field (an ordinary "unspecified socket"), so it - # must be present regardless of what the registry says; see #492, #503. + # must be present regardless of what the registry says; see #492. # # That is a statement about the *scraped table*, not about the number being # unsourced. XSIS 028112 Appendix D (cited under RANGES below) reserves it @@ -252,7 +252,7 @@ #: Range names from :data:`RANGES` above that name an allocation *policy* for #: the pool (who may claim a code, or that nobody has) rather than a specific -#: assigned protocol. The owner's original ruling on #775/#847 held +#: assigned protocol. The owner's original ruling on #775/#841 held #: ``Registered by Xerox`` out of this set as a real ownership fact rather #: than a placeholder. #775's final round converts it too, so this set no #: longer decides *whether* a range registers -- only *how its name is diff --git a/pcapkit/vendor/pcapng/tls_key_label.py b/pcapkit/vendor/pcapng/tls_key_label.py index 3f550dac2a..4e4655b9ca 100644 --- a/pcapkit/vendor/pcapng/tls_key_label.py +++ b/pcapkit/vendor/pcapng/tls_key_label.py @@ -24,7 +24,7 @@ __all__ = ['TLSKeyLabel'] #: Hand-carried comment for the ``RSA`` member, copied verbatim from the text -#: GitHub issue #883 gave it in +#: GitHub issue #882 gave it in #: :class:`pcapkit.protocols.misc.pcapng.TLSKeyLabel` -- ``RSA`` has no row in #: :attr:`TLSKeyLabel.LINK`'s registry to generate a comment from (``grep -c #: RSA`` on the fetched CSV is 0), so it is carried across rather than @@ -134,7 +134,7 @@ def process(self, data: 'list[str]') -> 'tuple[list[str], list[str]]': pref = f"{renm} = '{label}'" # NOTE: mirrors the ``# nosec B105`` bandit suppression every # such member already carries in - # :class:`pcapkit.protocols.misc.pcapng.TLSKeyLabel` as of #883 -- + # :class:`pcapkit.protocols.misc.pcapng.TLSKeyLabel` as of #882 -- # bandit's B105 (hardcoded password string) flags a string # constant assigned to a name containing ``SECRET``, which eight # of these ten labels' names do. diff --git a/pcapkit/vendor/reg/apptype/apptype.py b/pcapkit/vendor/reg/apptype/apptype.py index d7a5b24bf9..9b13a4ce52 100644 --- a/pcapkit/vendor/reg/apptype/apptype.py +++ b/pcapkit/vendor/reg/apptype/apptype.py @@ -136,12 +136,12 @@ class TransportProtocol(EnumLookup, IntEnum): # ``undefined`` is declared explicitly as ``0`` and every other member # is ``auto()``, which continues from the preceding explicit value # rather than needing its own ``_start_ = 0`` to begin there; the - # owner's own ruling on this issue (#860) is explicit that + # a ruling given in review of the work for #860 is explicit that # ``undefined`` stays a direct ``0`` for that reason: "undefined # direct uses 0. then other real transport use auto. so we don't have # to define a _start_ and the undefined declaration is explicit." - # GitHub issue #808 already dropped the ``IntFlag`` base once - # nothing built a composite, and GitHub PR #836's ruling later + # GitHub issue #808 already dropped the ``IntFlag`` base once nothing + # built a composite, and a ruling given in review of that work later # retired ``|``-composite decoding entirely: "since it's no longer a # Flag, `|` joined values are no longer parsed and accepted, we will # treat it as a whole, instead of splitting." With no decoding left to @@ -154,7 +154,7 @@ class TransportProtocol(EnumLookup, IntEnum): # bare -- into anything narrower than the whole value it already is, on # either numbering; a plain ``dict.get`` lookup cannot tell a composed # ``int`` from any other one that happens to equal it. Treating every - # int as a whole is exactly what #836's ruling above asks for, and it + # int as a whole is exactly what that ruling above asks for, and it # is also why this renumbering changes what specific ints mean, not # only what composed ones do: # ``tcp | udp`` (``3``) used to name no registry and now resolves as @@ -201,22 +201,23 @@ def get(cls, key: 'int | str', default: 'Any' = NO_DEFAULT) -> 'TransportProtoco on these circumstances."* A stdlib ``E['nosuch']`` raises :exc:`KeyError`, and #923's census of the 127 concrete :class:`~pcapkit.corekit.enum.EnumLookup` subclasses -- taken before - #921 re-parented this class, so this class is not among them -- found - 119 already answering a name miss that way against 5 answering with - :exc:`ValueError`. Those 5 are :class:`AppType` and its four transport - registries, and they land there only because their own ``get()`` takes - an :class:`int` port and never accepts a name at all, rather than from - any name-miss policy. So there was no policy here to preserve, and a - name miss now reaches the caller as + the phase-2 re-parenting moved this class, so it is not among them -- + found 119 already answering a name miss that way against 5 answering + with :exc:`ValueError`. Those 5 are :class:`AppType` and its four + transport registries, and they land there only because their own + ``get()`` takes an :class:`int` port and never accepts a name at all, + rather than from any name-miss policy. So there was no policy here + to preserve, and a name miss now reaches the caller as :exc:`~pcapkit.utilities.exceptions.EnumKeyError` from the base. - Maintainer ruling on GitHub PR #836 -- "Do not allow extension of - TransportProtocol at all" -- is untouched by that: the refusal is still - a refusal and still mints nothing, only its exception class moved. + Maintainer ruling given in review of the work for #808 -- "Do not + allow extension of TransportProtocol at all" -- is untouched by + that: the refusal is still a refusal and still mints nothing, only + its exception class moved. The base is a :class:`classmethod` (:meth:`~pcapkit.corekit.enum.EnumLookup.get`), so this override moves from :class:`staticmethod` to :class:`classmethod` to - delegate at all -- the same move GitHub issue #908 and #915 made for + delegate at all -- the same move GitHub issue #908 made for :meth:`~pcapkit.const.http.method.Method.get`. Grepped every call site in this tree for GitHub issue #877: all call this method by name, none take it as a bare callable or introspect ``__func__``, @@ -256,19 +257,20 @@ def get(cls, key: 'int | str', default: 'Any' = NO_DEFAULT) -> 'TransportProtoco """ if isinstance(key, str): return super().get(key.lower(), default) - # NOTE: maintainer ruling on this PR (#836): "Do not allow extension - # of TransportProtocol at all." A name that is not a declared member - # used to mint a brand-new one here, at ``max_val + 1`` (before that, - # ``max_val * 2``) -- an unbounded, ever-growing set of transport - # protocols nothing ever asked for. There is nothing left to walk - # now: it is simply refused, exactly like any other unrecognised - # name -- including one spelling a composite, e.g. ``'tcp|udp'``. + # NOTE: maintainer ruling given in review of the work for #808: "Do + # not allow extension of TransportProtocol at all." A name that is + # not a declared member used to mint a brand-new one here, at + # ``max_val + 1`` (before that, ``max_val * 2``) -- an unbounded, + # ever-growing set of transport protocols nothing ever asked for. + # There is nothing left to walk now: it is simply refused, exactly + # like any other unrecognised name -- including one spelling a + # composite, e.g. ``'tcp|udp'``. # ``'|'`` used to be intercepted here on its own, so a composite in # disguise never got minted into a member whose own name lied about - # being a single transport; the owner's further ruling on this PR - # retired that special case along with the rest of the composite - # handling once TransportProtocol stopped being a Flag at all: - # "since it's no longer a Flag, `|` joined values are no longer + # being a single transport; a further ruling given in review of the + # work for #808 retired that special case along with the rest of the + # composite handling once TransportProtocol stopped being a Flag at + # all: "since it's no longer a Flag, `|` joined values are no longer # parsed and accepted, we will treat it as a whole, instead of # splitting." A ``'|'``-joined name is therefore not special any # more -- it is simply not the name of a declared member, and gets @@ -479,10 +481,10 @@ def _dispatch(cls, key: 'int', proto: 'TransportProtocol | str | int') -> 'Type[ the same as any caller passing a literal port-transport bitmask -- falls through to ``int.__or__`` and returns a bare :class:`int` rather than a member. Never split back into the - transports its bits would each name -- owner ruling on this PR - (#836) -- so it is looked up as the one whole value it - already is, exactly like any other bare int: refused when - that whole value names no registry, resolved when it + transports its bits would each name -- a ruling given in review + of the work for #808 -- so it is looked up as the one whole + value it already is, exactly like any other bare int: refused + when that whole value names no registry, resolved when it happens to equal one instead. Since GitHub issue #860 moved this class off power-of-two spacing, a hand-built composite is no longer guaranteed to be the former -- see @@ -530,11 +532,12 @@ def _dispatch(cls, key: 'int', proto: 'TransportProtocol | str | int') -> 'Type[ # either numbering. A genuine member reaching this point is # ``undefined`` -- the four real transports would already have # resolved above, and :meth:`TransportProtocol.get` cannot mint - # anything else, per this PR's own maintainer ruling against - # extending TransportProtocol at all -- and a bare :class:`int` - # reaches here whenever it matches no real member's value, e.g. a - # stray bit like ``17``. A composite built by hand used to reach - # here just as reliably, since no combination of the old power-of- + # anything else, per the maintainer ruling given in review of the + # work for #808, against extending TransportProtocol at all -- and + # a bare :class:`int` reaches here whenever it matches no real + # member's value, e.g. a stray bit like ``17``. A composite built by + # hand used to reach here just as reliably, since no combination of + # the old power-of- # two bits ever equalled a single real member's value; that is no # longer true under this class's current sequential numbering -- # ``TransportProtocol.tcp | TransportProtocol.udp`` (``3``) is @@ -545,10 +548,10 @@ def _dispatch(cls, key: 'int', proto: 'TransportProtocol | str | int') -> 'Type[ # and naming every transport whose bit was set -- the fix for # GitHub issue #759, where resolving a composite by picking its # lowest set bit dispatched every one containing ``tcp`` into the - # TCP registry regardless of what else it named. The owner's - # further ruling on this PR (#836) retired that decoding along with - # the rest of the composite handling: "since it's no longer a Flag, - # `|` joined values are no longer parsed and accepted, we will + # TCP registry regardless of what else it named. A further ruling + # given in review of the work for #808 retired that decoding along + # with the rest of the composite handling: "since it's no longer a + # Flag, `|` joined values are no longer parsed and accepted, we will # treat it as a whole, instead of splitting." So a composite's bits # are never decoded looking for a partial answer any more, on # either numbering -- it is looked up as the one whole value it diff --git a/pcapkit/vendor/reg/ethertype.py b/pcapkit/vendor/reg/ethertype.py index 93a5992642..de08a19136 100644 --- a/pcapkit/vendor/reg/ethertype.py +++ b/pcapkit/vendor/reg/ethertype.py @@ -36,7 +36,7 @@ class EtherType(Vendor): #: carrying no real assignment -- a company holding the block but naming #: nothing (``DEC Unassigned``), or a range the list says is dead/invalid #: outright -- rather than a proprietary protocol's real name. The owner's - #: original ruling on #775/#847 held these two out as the only rows to + #: original ruling on #775/#841 held these two out as the only rows to #: convert to :meth:`~pcapkit.corekit.enum.EnumRegistry._unregistered_ #: member`, on the theory that a proprietary protocol's company name IS #: the final concrete name for every other row. #775's final round diff --git a/pcapkit/vendor/reg/linktype.py b/pcapkit/vendor/reg/linktype.py index ba0fe4ad2f..d0e8064066 100644 --- a/pcapkit/vendor/reg/linktype.py +++ b/pcapkit/vendor/reg/linktype.py @@ -69,7 +69,7 @@ def process(self, data: 'list[Tag]') -> 'tuple[list[str], list[str]]': ] # Value-aware guard for the ``sink`` decision below (GitHub issue #852, - # a follow-up to #848/#844). A row's notes mentioning "legacy" is + # a follow-up to #844). A row's notes mentioning "legacy" is # tcpdump's only textual signal for *which* member of a duplicated # value is the deprecated one, but that word can appear in a row's # notes for an unrelated reason -- e.g. referencing some other, From c65f160745923eb1fb68978e2df1796b2de127ff Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Thu, 1 Oct 2026 21:35:04 -0400 Subject: [PATCH 2/9] docs(corekit): cite #719, not #937, for the AbsentType ruling AbsentType's docstring attributed the owner's "document it as private type/class... not for public use is enough" quote to #937. #937 itself quotes that ruling verbatim under "The owner's ruling, verbatim (from #719)" -- it re-attributes rather than originates it. Per the house rule to cite the issue a ruling was settled on (docs/source/contributing/conventions/documentation.rst:196-200), point the attribution at #719 and re-wrap the paragraph to the file's existing ~78-column width. The neighbouring, unrelated #937 citation describing what #937 did to sentinel naming is untouched. tests/corekit/ passes (400 passed, 16 skipped) against this worktree's own pcapkit (confirmed via pcapkit.__file__); pylint on the file is 9.77/10, unchanged by this edit -- the one finding is a pre-existing, unrelated too-few-public-methods warning on NoValueType. --- pcapkit/corekit/sentinels.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pcapkit/corekit/sentinels.py b/pcapkit/corekit/sentinels.py index 8c9407b72e..9b046038d6 100644 --- a/pcapkit/corekit/sentinels.py +++ b/pcapkit/corekit/sentinels.py @@ -456,13 +456,13 @@ class AbsentType: normalised every sentinel *object* to SCREAMING_SNAKE and dropped it, so this pair now reads as CamelCase/SCREAMING_SNAKE like their two siblings and privacy is no longer signalled by the name at all. The owner's ruling - on #937, verbatim: *"we can change* ``_ABSENT`` *to* ``ABSENT`` *just - document it as private type/class in the documentation and not for public - use is enough."* So this class and :data:`ABSENT` stay exactly as private - as they were: nothing outside :mod:`pcapkit.protocols.protocol` reads - :data:`ABSENT`, from here or from there, and neither this module's nor - that module's :attr:`__all__` names either one. This docstring, and the - "Naming a Sentinel" section of + on GitHub issue #719, verbatim: *"we can change* ``_ABSENT`` *to* + ``ABSENT`` *just document it as private type/class in the documentation + and not for public use is enough."* So this class and :data:`ABSENT` stay + exactly as private as they were: nothing outside + :mod:`pcapkit.protocols.protocol` reads :data:`ABSENT`, from here or from + there, and neither this module's nor that module's :attr:`__all__` names + either one. This docstring, and the "Naming a Sentinel" section of :file:`docs/source/contributing/conventions/sentinel-convention.rst`, are what now records that fact in place of the leading underscore. From f334953a8a95201750e73217138c918c058bdfec Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Thu, 1 Oct 2026 22:24:40 -0400 Subject: [PATCH 3/9] docs(tests): re-point six ruling citations at their actual threads, per #719 Swept tests/ for owner-ruling quotes attributed to the wrong GitHub thread, the same defect class #719 fixed under pcapkit/. Confirmed each by grepping the quote's distinctive text against the cited thread's body/comments; a hit elsewhere is a re-quote or a different thread's own words, not the source. - test_sentinel_exports_unit.py: the already-reported #937->#719 fix for AbsentType's privacy ruling. - test_enum_lookup_reparent_930_unit.py (4 sites) and test_mh_unit.py: "I prefer (2) directly" and the question that drew it are in pull request #940's thread, not issue #935 -- #935 only carries the first ruling ("I lean on 1"). - test_vendor_snapshot_restore_unit.py: the contextlib/atomic-write ruling is in pull request #873's thread; issue #872 has zero comments. - test_vendor_reg_apptype_generator_unit.py (2 sites): the "undefined direct uses 0" ruling is in pull request #874's thread, not issue #860 or #770. - test_const_enum_no_mint.py (2 sites): the mint/unmint criterion was settled on pull request #847 and confirmed on #775 -- the reverse of what the text said, per #861's own description of the same ruling; and "Q1 - bare it is." is pull request #838's thread, not #775's. One occurrence left unresolved rather than guessed at: the "Preserve each branch's existing name argument..." quote (4 sites in test_const_enum_no_mint.py, attributed to "#775's final round") does not appear verbatim in #775, #847, or #878 (the implementing PR) by body, comments, review comments, or commit message -- only a paraphrase in #878's own PR description/commit message, which is the author's prose rather than a quoted ruling. Flagged for the owner rather than fixed. tests/corekit/, tests/vendor/, tests/const/ pass (400/16, 118, 299 respectively, pcapkit.__file__ confirmed inside this worktree); tests/protocols/internet/test_mh_unit.py passes standalone (52/0) -- the full directory has 5 unrelated pre-existing failures from ungenerated examples/captures/ fixtures, untouched by this change. Refs #719 --- tests/const/test_const_enum_no_mint.py | 25 ++++++++------- .../test_enum_lookup_reparent_930_unit.py | 32 +++++++++---------- tests/corekit/test_sentinel_exports_unit.py | 6 ++-- tests/protocols/internet/test_mh_unit.py | 20 ++++++------ .../test_vendor_reg_apptype_generator_unit.py | 20 ++++++------ .../test_vendor_snapshot_restore_unit.py | 10 +++--- 6 files changed, 57 insertions(+), 56 deletions(-) diff --git a/tests/const/test_const_enum_no_mint.py b/tests/const/test_const_enum_no_mint.py index f62d2ccb5a..2daf7ff87b 100644 --- a/tests/const/test_const_enum_no_mint.py +++ b/tests/const/test_const_enum_no_mint.py @@ -24,10 +24,11 @@ Tier 1 fixed the *mechanism* (a lookup should never mint); it did not decide, registry by registry, which of the remaining ``_missing_`` bodies mint -something worth keeping. That is tier 2, decided on #775 and carried out on -#847: the owner's ruling, verbatim, is *"a final concrete assigned name -> -mint; a notation for the readers -> unmint"*. Applied registry by registry to -every ``_missing_`` that still called :func:`~aenum.extend_enum` -- measured +something worth keeping. That is tier 2; the owner's mint/unmint criterion, +settled on GitHub pull request #847 and confirmed on #775, is verbatim: *"a +final concrete assigned name -> mint; a notation for the readers -> unmint"*. +Applied registry by registry to every ``_missing_`` that still called +:func:`~aenum.extend_enum` -- measured at exactly 89 modules by an AST walk over ``pcapkit/const/*.py`` (not the "~92" an earlier pass in this programme estimated) -- the ruling converted 82 of them outright (172 branches, :data:`RULING_CONVERTED_REGISTRIES` below), @@ -1204,15 +1205,15 @@ def _is_hex_suffixed_unregistered_name(name_arg: 'Optional[ast.expr]') -> bool: class UnregisteredMemberNameIsBareTests(unittest.TestCase): - """#775's Q1 follow-up, the maintainer's ruling verbatim: *"Q1 - bare it - is."* Asked whether the non-minting path should honour the registry's - own ``unassigned``/``reserved`` name directly or keep appending the - numeric value, he chose the bare name -- safe precisely because a + """GitHub pull request #838's Q1 follow-up, the maintainer's ruling verbatim: + *"Q1 - bare it is."* Asked whether the non-minting path should honour the + registry's own ``unassigned``/``reserved`` name directly or keep appending + the numeric value, he chose the bare name -- safe precisely because a pseudo-member built by :meth:`_unregistered_member` never enters - ``__members__``/``_member_map_``/``_value2member_map_``, so two - same-named pseudo-members (e.g. ``Chunk._unregistered_member(20, - 'Unassigned')`` and ``(70, 'Unassigned')``) cannot collide the way two - *minted* ``extend_enum`` members with the same name would. + ``__members__``/``_member_map_``/``_value2member_map_``, so two same-named + pseudo-members (e.g. ``Chunk._unregistered_member(20, 'Unassigned')`` and + ``(70, 'Unassigned')``) cannot collide the way two *minted* + ``extend_enum`` members with the same name would. This walks every generated :mod:`pcapkit.const` module by AST -- rather than pinning one example -- and asserts that every diff --git a/tests/corekit/test_enum_lookup_reparent_930_unit.py b/tests/corekit/test_enum_lookup_reparent_930_unit.py index 34973fcb23..176ed5fd8b 100644 --- a/tests/corekit/test_enum_lookup_reparent_930_unit.py +++ b/tests/corekit/test_enum_lookup_reparent_930_unit.py @@ -27,10 +27,10 @@ ``mypy``'s ``[override]`` check plus ``pylint``'s ``arguments-differ`` both flagged the resulting shape mismatch, silenced with a suppression. GitHub issue #935 first answered that on the owner's ruling, verbatim: *"I lean on 1"* -- widen both signatures to accept -``default`` and delete the suppression. Asked, on the same issue, *"why must we have the -two overrides tho? cant they directly fall back to the base class's?"*, the owner's final -ruling went further, verbatim: *"I prefer (2) directly"* -- deleting both overrides -outright rather than widening them. +``default`` and delete the suppression. Asked, on GitHub pull request #940 -- which was +implementing that widening -- *"why must we have the two overrides tho? cant they directly +fall back to the base class's?"*, the owner's final ruling went further, verbatim: *"I +prefer (2) directly"* -- deleting both overrides outright rather than widening them. Measured before acting on that final ruling: neither override ever minted an alias -- ``__members__`` and ``list(cls)`` agree at 6 and 4 -- so what each docstring called @@ -198,9 +198,9 @@ def test_esp_status(self) -> None: self.assertEqual(len(list(ESPStatus)), 6) def test_fast_binding_acknowledgment_status(self) -> None: - """GitHub issue #935 later deleted its kept ``get`` override outright - (the owner's ruling, verbatim: *"I prefer (2) directly"*), so the - decorator this once pinned no longer exists to pin -- + """GitHub pull request #940 later deleted its kept ``get`` override + outright (the owner's ruling, verbatim: *"I prefer (2) directly"*), so + the decorator this once pinned no longer exists to pin -- :class:`AllSevenInheritTheBareClassmethodTests` now covers this class alongside the other six.""" from aenum import IntEnum @@ -450,10 +450,10 @@ class was ``KeptOverrideQuietnessTests`` while both classes still carried their own ``get``, first through #930's re-parenting and briefly again through GitHub issue #935's first attempt, which widened that override to accept ``default`` rather than delete it. The owner's - final ruling on #935 deleted both outright instead, verbatim: *"I - prefer (2) directly"*. What this class pins did not change with that - deletion -- the quiet raise -- only *how* it is produced: through - :meth:`~pcapkit.corekit.enum.EnumLookup.get` + final ruling, given on GitHub pull request #940, deleted both outright + instead, verbatim: *"I prefer (2) directly"*. What this class pins did not + change with that deletion -- the quiet raise -- only *how* it is produced + through :meth:`~pcapkit.corekit.enum.EnumLookup.get` (:mod:`pcapkit.corekit.enum`) directly now, rather than through an override that reconciled itself onto the base's shape. @@ -573,12 +573,12 @@ class AllSevenInheritTheBareClassmethodTests(unittest.TestCase): Five were always this way -- pure re-parents, having defined no ``get`` of their own to begin with. The other two, :class:`FastBindingAcknowledgmentStatus` and - :class:`IPv6AddressPrefixCode`, joined them only at GitHub issue #935's + :class:`IPv6AddressPrefixCode`, joined them only at GitHub pull request #940's final revision: their own kept ``get`` stayed a :class:`staticmethod` - through #930's re-parenting and briefly again through #935's first - attempt (which widened it to accept ``default`` rather than delete it), - and only the owner's final ruling there -- verbatim, *"I prefer (2) - directly"* -- deleted it outright, collapsing the seven-way split this + through #930's re-parenting and briefly again through #935's first attempt + (which widened it to accept ``default`` rather than delete it), and only + the owner's final ruling there -- verbatim, *"I prefer (2) directly"* -- + deleted it outright, collapsing the seven-way split this class used to test as five-plus-two into one uniform case. This class was named for the five alone before that ruling, and :class:`ReparentedBasesTests` pinned the other two's surviving diff --git a/tests/corekit/test_sentinel_exports_unit.py b/tests/corekit/test_sentinel_exports_unit.py index 241a1a8f9e..2cfd656563 100644 --- a/tests/corekit/test_sentinel_exports_unit.py +++ b/tests/corekit/test_sentinel_exports_unit.py @@ -46,9 +46,9 @@ ``NoValue`` to ``NO_VALUE`` and ``_Absent`` to ``ABSENT``, the latter also dropping its leading underscore -- so every instance name agrees on one casing. Privacy for what is now ``ABSENT`` stopped being signalled by the name at all and became -documentation-only, per the owner's ruling on #937: *"we can change* ``_ABSENT`` *to* -``ABSENT`` *just document it as private type/class in the documentation and not for -public use is enough."* +documentation-only, per the owner's ruling on GitHub issue #719, verbatim: +*"we can change* ``_ABSENT`` *to* ``ABSENT`` *just document it as private +type/class in the documentation and not for public use is enough."* :meth:`SentinelExportTests.test_the_private_sentinel_is_exported_neither_way` is what now pins that privacy under the new name, since the mechanical underscore signal it used to double-check is gone. diff --git a/tests/protocols/internet/test_mh_unit.py b/tests/protocols/internet/test_mh_unit.py index be3cbe59f9..15eb55c7a2 100644 --- a/tests/protocols/internet/test_mh_unit.py +++ b/tests/protocols/internet/test_mh_unit.py @@ -1618,16 +1618,16 @@ def test_mh_get_default_works_uniformly_through_the_inherited_method(self) -> No ``TypeError: get() takes 1 positional argument but 2 were given``. GitHub issue #935's first ruling, verbatim *"I lean on 1"*, widened both signatures to accept ``default`` rather than delete them; asked - next *"why must we have the two overrides tho? cant they directly - fall back to the base class's?"*, the owner's final ruling went - further, verbatim: *"I prefer (2) directly"* -- deleting both - overrides outright. Both classes now inherit ``get`` from the base - exactly as :class:`LMAAddressCode` and :class:`LocalizedRoutingStatus` - -- the two pure re-parents already in this module -- always have, so - ``default`` now works the same way on all four, uniformly, because - there is only one implementation left to call. This test fails with - the ``TypeError`` above against the tree at 382375811, before either - of #935's rulings landed. + next, on GitHub pull request #940, *"why must we have the two + overrides tho? cant they directly fall back to the base class's?"*, + the owner's final ruling there went further, verbatim: *"I prefer (2) + directly"* -- deleting both overrides outright. Both classes now + inherit ``get`` from the base exactly as :class:`LMAAddressCode` and + :class:`LocalizedRoutingStatus` -- the two pure re-parents already in + this module -- always have, so ``default`` now works the same way on + all four, uniformly, because there is only one implementation left to + call. This test fails with the ``TypeError`` above against the tree at + 382375811, before either ruling landed. """ import sys diff --git a/tests/vendor/test_vendor_reg_apptype_generator_unit.py b/tests/vendor/test_vendor_reg_apptype_generator_unit.py index ff777a2012..335300be13 100644 --- a/tests/vendor/test_vendor_reg_apptype_generator_unit.py +++ b/tests/vendor/test_vendor_reg_apptype_generator_unit.py @@ -61,9 +61,9 @@ with no bare literal left anywhere, no member's wrapper was load-bearing against a mypy error any more, since an ``auto()``-valued member infers as ``Any``, which is assignable to ``TransportProtocol`` with no cast at all. -The owner's final ruling on this issue reinstated the original shape -instead, verbatim: *"undefined direct uses 0. then other real transport -use auto. so we don't have to define a _start_ and the undefined +The owner's final ruling, given on GitHub pull request #874, reinstated the +original shape instead, verbatim: *"undefined direct uses 0. then other real +transport use auto. so we don't have to define a _start_ and the undefined declaration is explicit."* So ``undefined`` is a direct, explicit ``0`` again, and ``tcp``/``udp``/``sctp``/``dccp`` continue from it via plain ``auto()`` with no ``_start_`` needed at all -- ``auto()`` picks up the @@ -225,13 +225,13 @@ def test_undefined_member_is_cast_rather_than_a_bare_literal(self) -> None: literal left anywhere, no member's wrapper was load-bearing against a mypy error any more (measured at the time: unwrapping ``undefined`` alone stayed mypy-clean, identically to unwrapping ``tcp`` instead). - The owner's final ruling on the issue reinstated the original - shape, verbatim: *"undefined direct uses 0. then other real - transport use auto. so we don't have to define a _start_ and the - undefined declaration is explicit."* So ``undefined`` is a direct, - explicit ``cast('TransportProtocol', 0)`` again, exactly as #770 - first shaped it, and this test's own check (and #770's asymmetry) - are both back to describing the tree as it actually ships. + The owner's final ruling, given on GitHub pull request #874, + reinstated the original shape, verbatim: *"undefined direct uses 0. + then other real transport use auto. so we don't have to define a + _start_ and the undefined declaration is explicit."* So ``undefined`` + is a direct, explicit ``cast('TransportProtocol', 0)`` again, exactly + as #770 first shaped it, and this test's own check (and #770's + asymmetry) are both back to describing the tree as it actually ships. Measured directly, to prove the load-bearing claim rather than assert it: with ``undefined`` unwrapped to a bare ``0`` (everything diff --git a/tests/vendor/test_vendor_snapshot_restore_unit.py b/tests/vendor/test_vendor_snapshot_restore_unit.py index 99cbec746b..75acf310c9 100644 --- a/tests/vendor/test_vendor_snapshot_restore_unit.py +++ b/tests/vendor/test_vendor_snapshot_restore_unit.py @@ -4,11 +4,11 @@ GitHub issue #872. Rounds 4-8 made :meth:`~pcapkit.vendor.default.Vendor.__init__`'s own write atomic -- a temp-file-then-:func:`os.replace` at the point of the write, with permission -matching for the replacement. The owner's ruling on that, verbatim: *"I -prefer we use contextlib over manually manage the temp file deletion based -on pure best intent (try-finally) and for atomic writing, an easier path is -simply keep a copy before running the sub-vendor and revert if anything -failed."* +matching for the replacement. The owner's ruling on GitHub pull request +#873, verbatim: *"I prefer we use contextlib over manually manage the temp +file deletion based on pure best intent (try-finally) and for atomic +writing, an easier path is simply keep a copy before running the sub-vendor +and revert if anything failed."* That supersedes the atomic write entirely rather than adjusting it. :meth:`~pcapkit.vendor.default.Vendor._write_atomic` no longer exists; From 0a2fcc03314dceae0ba247b140fa3ae94d13a37c Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Thu, 1 Oct 2026 23:39:36 -0400 Subject: [PATCH 4/9] docs(tests): paraphrase four fabricated or altered owner quotations (#719) Per #719's citation ruling (de-quote, never reproduce a verbatim quote that may have come from outside GitHub): - test_const_enum_no_mint.py (4 sites): a quotation attributed to "the owner's ruling, verbatim" never appears in #775, #847, #861 or #878 (or anywhere in the repo's comment corpus). Replaced with a paraphrase attributed to PR #878's own body, which carries the real design note in different words. - test_sentinel_exports_unit.py / test_const_registry_protocol.py: a quote attributed to #911 silently dropped half of what the owner wrote on #719 and swapped `__all__` for "users". Replaced with a paraphrase naming #719 as where it was settled and #911 as the issue that carried it out. - test_const_enum_no_mint.py / test_const_enum_builtin_parity.py (4 sites): a "verbatim" quote of #860 silently corrected the owner's typo ("entires" -> "entries"). Paraphrased, which drops the question of reproducing or flagging the typo. - test_enum_lookup_reparent_930_unit.py: "the owner's final ruling there" had #935 as its nearest antecedent instead of #940; named #940 explicitly and paraphrased the adjacent quote. Verified: ast.parse and reST markup pairing clean on every touched file; tests/const (299 tests) and the targeted pytest sweep of all touched files (261 passed, 2013 subtests) are green. tests/corekit's full discover run shows 5 pre-existing failures in test_sentinel_exports_unit.py, confirmed identical on the unedited originals -- a cross-file test-order dependency unrelated to this change. --- tests/const/test_const_enum_builtin_parity.py | 9 ++-- tests/const/test_const_enum_no_mint.py | 52 +++++++++---------- tests/const/test_const_registry_protocol.py | 8 +-- .../test_enum_lookup_reparent_930_unit.py | 4 +- tests/corekit/test_sentinel_exports_unit.py | 9 ++-- 5 files changed, 40 insertions(+), 42 deletions(-) diff --git a/tests/const/test_const_enum_builtin_parity.py b/tests/const/test_const_enum_builtin_parity.py index 03588af7cf..fc9500e3b0 100644 --- a/tests/const/test_const_enum_builtin_parity.py +++ b/tests/const/test_const_enum_builtin_parity.py @@ -652,11 +652,10 @@ class ConstEnumRegisterFallbackTests(unittest.TestCase): transport subclasses) were, until GitHub issue #860 step 2, the last registries still living that divergence for an *unrecognised* value (every numeric registry had already lost it under #775/#847's ruling). - The owner's #860 ruling retired it for all of them, verbatim: *"I think - we should not mint on get still actually. For all three, only IANA - registered ones are legit values and we need register to properly - create new entries. get will not have sufficient information to create - new ones."* Stated for the three PR 1 converted, but the reasoning is + The owner's #860 ruling retired it for all of them: only IANA-registered + values are legitimate, so creating a new registry entry is + :meth:`register`'s job alone, and ``get()`` has no way to supply what + that would take. Stated for the three PR 1 converted, but the reasoning is unconditional and PR 2 applies it to ``AppType`` identically. Concretely, :class:`Command` needs :attr:`~pcapkit.const.ftp.command.Command.feat`/ :attr:`~pcapkit.const.ftp.command.Command.desc`/ diff --git a/tests/const/test_const_enum_no_mint.py b/tests/const/test_const_enum_no_mint.py index 2daf7ff87b..2d55ac94ac 100644 --- a/tests/const/test_const_enum_no_mint.py +++ b/tests/const/test_const_enum_no_mint.py @@ -85,10 +85,9 @@ initially left minting pending the owner's ruling (this measurement's own report flagged them as genuinely ambiguous under the criterion, since nothing about them is a manufactured placeholder). The owner's ruling on -#860, verbatim: *"I think we should not mint on get still actually. For all -three, only IANA registered ones are legit values and we need register to -properly create new entries. get will not have sufficient information to -create new ones."* That reasoning reaches ``get()`` as well as +#860 settled it: only IANA-registered values are legitimate, and creating a +new registry entry is :meth:`register`'s job alone -- nothing else has +enough information to supply one. That reasoning reaches ``get()`` as well as ``_missing_`` -- :class:`Command` needs ``feat``/``desc``/``type``/``conf`` and :class:`Method` needs ``safe``/``idempotent``, neither of which a bare wire string carries -- so both classes' own ``get()`` (a second, independent @@ -144,10 +143,10 @@ every one of :class:`~pcapkit.const.reg.ethertype.EtherType`'s 52 still- minting range branches, and :class:`~pcapkit.const.ipx.socket.Socket`'s one (``Registered by Xerox``), now convert to :meth:`~pcapkit.corekit.enum. -EnumRegistry._unregistered_member` too -- the owner's ruling, verbatim: -*"Preserve each branch's existing name argument exactly as the current code -produces it -- this change is about not registering, not about renaming -anything."* So each keeps the hex-suffixed name it always rendered +EnumRegistry._unregistered_member` too -- PR #878 scoped the change this way: +preserve each branch's existing name argument exactly as the current code +produces it, since this is about not registering rather than about renaming +anything. So each keeps the hex-suffixed name it always rendered (``Xyplex_0x0888``, not a bare ``Xyplex``) even though it no longer registers -- neither is "mixed" any more, both are wholly converted like the 82 in :data:`RULING_CONVERTED_REGISTRIES`, and :class:`EtherTypeMixedMintTests` @@ -590,10 +589,10 @@ def _first_unassigned_value(cls: 'type') -> 'int': } #: The one probe the original ruling held out as "a real ownership fact, keep #: minting" -- Xyplex, 0x0888. #775's final round converts it too, preserving -#: the hex-suffixed name exactly as the crawler always rendered it (the -#: owner's ruling, verbatim: *"Preserve each branch's existing name argument -#: exactly as the current code produces it -- this change is about not -#: registering, not about renaming anything."*), so this now pins the +#: the hex-suffixed name exactly as the crawler always rendered it (PR #878 +#: scoped it that way: preserving each branch's existing name argument +#: exactly as the current code produces it, since this is about not +#: registering rather than about renaming anything), so this now pins the #: opposite of what its name suggests: that the formerly-kept probe no #: longer mints either. Kept as its own constant, distinct from #: :data:`ETHERTYPE_UNASSIGNED_PROBES`, because :class:`EtherTypeMixedMintTests` @@ -1286,10 +1285,10 @@ class UnregisteredMemberNameIsBareTests(unittest.TestCase): GitHub issue #775's final round converts the last 53 minting branches -- all 52 of :class:`~pcapkit.const.reg.ethertype.EtherType`'s and :class:`~pcapkit.const.ipx.socket.Socket`'s one -- and deliberately keeps - each one's hex-suffixed name unchanged (the owner's ruling, verbatim: - *"Preserve each branch's existing name argument exactly as the current - code produces it -- this change is about not registering, not about - renaming anything."*). That is the exact manufactured, value-suffixed + each one's hex-suffixed name unchanged (PR #878 scoped it this way: + preserving each branch's existing name argument exactly as the current + code produces it, since this is about not registering rather than + renaming anything). That is the exact manufactured, value-suffixed shape :func:`is_manufactured` exists to flag -- unlike ``'%s_unknown' % namespace`` above, the substituted operand here really is ``value``, via ``hex(value)[2:].upper().zfill(4)``. Flagging it anyway would be a false @@ -1536,10 +1535,10 @@ def test_formerly_attributed_vendor_block_no_longer_mints(self) -> None: exact probe (``Xyplex``, 0x0888) used to prove the ruling *kept* minting a real attributed name; GitHub issue #775's final round converts it, preserving the hex-suffixed name exactly as the crawler - always rendered it -- the owner's ruling, verbatim: *"Preserve each + always rendered it -- PR #878 scoped it this way: preserve each branch's existing name argument exactly as the current code - produces it -- this change is about not registering, not about - renaming anything."* Same shape as + produces it, since this is about not registering rather than + renaming anything. Same shape as :meth:`test_unassigned_rows_do_not_mint` above, just for the one probe that used to be the exception.""" from pcapkit.const.reg.ethertype import EtherType @@ -1872,11 +1871,10 @@ class BespokeOpenVocabularyUnmintConvertedTests(unittest.TestCase): (``Unassigned_%d``, ``Unknown_%d``); each minted the literal, exact string it was asked to resolve, as its own name. That initially read as a case for keeping them minting (the label was never manufactured), but - the owner's ruling on #860 settled it the other way, verbatim: *"I think - we should not mint on get still actually. For all three, only IANA - registered ones are legit values and we need register to properly - create new entries. get will not have sufficient information to create - new ones."* Concretely: :class:`~pcapkit.const.ftp.command.Command` + the owner's ruling on #860 settled it the other way: only IANA-registered + values are legitimate, creating a new registry entry is + :meth:`register`'s job, and ``get()`` cannot supply what that would + take. Concretely: :class:`~pcapkit.const.ftp.command.Command` needs ``feat``/``desc``/``type``/``conf`` and :class:`~pcapkit.const.http.method.Method` needs ``safe``/``idempotent``, neither of which a bare wire string carries, so @@ -2362,9 +2360,9 @@ class AppTypeUnmintConvertedTests(unittest.TestCase): merely being imported, which does not exist here, since every one of these 766 spans mints only on an actual port lookup -- and the owner's ruling for the whole ``AppType`` family draws no distinction between a - real name and a placeholder: *"only IANA registered ones are legit - values and we need register to properly create new entries. get will - not have sufficient information to create new ones."* A lookup resolving + real name and a placeholder: only IANA-registered values are + legitimate, creating a new registry entry is :meth:`register`'s job, + and ``get()`` lacks the information to do it. A lookup resolving ``TCP(6010)`` after this PR therefore returns an *unregistered* ``x11`` member -- correct as a service name, but absent from ``__members__``/``_value2member_map_`` until someone calls diff --git a/tests/const/test_const_registry_protocol.py b/tests/const/test_const_registry_protocol.py index ddd959e935..d3f7fa3d3f 100644 --- a/tests/const/test_const_registry_protocol.py +++ b/tests/const/test_const_registry_protocol.py @@ -1360,10 +1360,10 @@ def test_type_is_the_dedicated_sentinel_class_and_only_the_object_is_exported(se """GitHub issue #911 reversed half of what this used to assert. It read ``assertIn('NoDefaultType', enum_module.__all__)`` -- the type - *and* the object were exported. The owner's ruling: *"we should ONLY - export the objects (like* ``NULL`` *) to users"*, so the type is out of - :attr:`__all__` while staying importable by its dotted path, which is - what the last assertion here pins. + *and* the object were exported. GitHub issue #719 settled that only the + object belongs in :attr:`__all__`, and #911 carried that out: the type + is out of :attr:`__all__` while staying importable by its dotted path, + which is what the last assertion here pins. """ import pcapkit.corekit.enum as enum_module diff --git a/tests/corekit/test_enum_lookup_reparent_930_unit.py b/tests/corekit/test_enum_lookup_reparent_930_unit.py index 176ed5fd8b..745408d008 100644 --- a/tests/corekit/test_enum_lookup_reparent_930_unit.py +++ b/tests/corekit/test_enum_lookup_reparent_930_unit.py @@ -577,8 +577,8 @@ class AllSevenInheritTheBareClassmethodTests(unittest.TestCase): final revision: their own kept ``get`` stayed a :class:`staticmethod` through #930's re-parenting and briefly again through #935's first attempt (which widened it to accept ``default`` rather than delete it), and only - the owner's final ruling there -- verbatim, *"I prefer (2) directly"* -- - deleted it outright, collapsing the seven-way split this + the owner's final ruling on #940 -- outright deletion over widening the + signature -- removed it, collapsing the seven-way split this class used to test as five-plus-two into one uniform case. This class was named for the five alone before that ruling, and :class:`ReparentedBasesTests` pinned the other two's surviving diff --git a/tests/corekit/test_sentinel_exports_unit.py b/tests/corekit/test_sentinel_exports_unit.py index 2cfd656563..23f1bae39b 100644 --- a/tests/corekit/test_sentinel_exports_unit.py +++ b/tests/corekit/test_sentinel_exports_unit.py @@ -1,9 +1,10 @@ # -*- coding: utf-8 -*- """GitHub issue #911: a sentinel exports its **object**, never its type. -The owner's ruling, verbatim: *"One thing about sentinel types and objects in the -library: we should ONLY export the objects (like* ``NULL`` *) to users."* That is -one rule with two directions, and the tree on ``origin/main`` broke it in both: +GitHub issue #719 is where the owner settled this: a module's :attr:`__all__` +should list a sentinel's object (like ``NULL``), never its type (like +``NullType``). Issue #911 carried that ruling out. That is one rule with two +directions, and the tree on ``origin/main`` broke it in both: * ``pcapkit.corekit.module.__all__`` was ``['NULL', 'NullType', 'ModuleDescriptor']`` -- the type is exported; @@ -31,7 +32,7 @@ ``_Absent``/``_AbsentType``, with a leading underscore. :class:`SentinelPopulationTests` pins the count and the doc together, so the next sentinel cannot be added to one without the other. ``ABSENT`` is private and stays out of :attr:`__all__` in both -directions, which is what the ruling means by "to users". +directions, consistent with the #719 ruling that only the object goes in. A follow-up to this same issue moved all four *definitions* into :mod:`pcapkit.corekit.sentinels`, per the owner's later ruling -- *"Okay one module From b137284020c2045caa6c79c647d3c2290be10a60 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 2 Oct 2026 00:02:41 -0400 Subject: [PATCH 5/9] test(vendor,corekit): fix a surviving fabricated ruling and a wrong citation (#719) - tests/vendor/test_ipx_socket_unit.py: the "owner's ruling" attribution for keeping the hex-suffixed Xerox name survived in this file after the prior commit removed the same false attribution from four sites in test_const_enum_no_mint.py. Reworded to credit PR #878's own design note, matching the wording already used at the repaired sites. - tests/corekit/test_sentinel_exports_unit.py: the docstring cited the #719 export ruling ("only export objects, not types") as grounds for keeping ABSENT out of __all__, but ABSENT is an object, so that ruling argues for including it, not excluding it. Re-grounded the sentence on the privacy ruling already quoted ~15 lines below instead, without re-quoting it. Both changes are prose-only: tokenizing each file before and after with comments and docstrings stripped produces identical token sequences. tests/vendor passes 118/118 except one pre-existing, test-order-dependent flake in test_vendor_snapshot_restore_unit.py (reproduces identically on the pre-edit tree); tests/project/test_conventions_doc_claims.py passes 38/38. --- tests/corekit/test_sentinel_exports_unit.py | 4 +++- tests/vendor/test_ipx_socket_unit.py | 9 +++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/corekit/test_sentinel_exports_unit.py b/tests/corekit/test_sentinel_exports_unit.py index 23f1bae39b..64a14a53b6 100644 --- a/tests/corekit/test_sentinel_exports_unit.py +++ b/tests/corekit/test_sentinel_exports_unit.py @@ -32,7 +32,9 @@ ``_Absent``/``_AbsentType``, with a leading underscore. :class:`SentinelPopulationTests` pins the count and the doc together, so the next sentinel cannot be added to one without the other. ``ABSENT`` is private and stays out of :attr:`__all__` in both -directions, consistent with the #719 ruling that only the object goes in. +directions -- not under the export ruling, which argues the opposite for an +object like this one, but under the privacy ruling quoted below, which treats a +sentinel documented as private as having no place in :attr:`__all__` at all. A follow-up to this same issue moved all four *definitions* into :mod:`pcapkit.corekit.sentinels`, per the owner's later ruling -- *"Okay one module diff --git a/tests/vendor/test_ipx_socket_unit.py b/tests/vendor/test_ipx_socket_unit.py index b502851766..d05ea035de 100644 --- a/tests/vendor/test_ipx_socket_unit.py +++ b/tests/vendor/test_ipx_socket_unit.py @@ -121,10 +121,11 @@ #: numeric suffix. ``Registered by Xerox`` was the one that stayed at the #: time, on the theory that it states a real ownership fact rather than a #: status placeholder -- #775's *final* round converts it too, keeping its -#: hex-suffixed name unchanged (the owner's ruling: preserve the existing -#: name argument exactly, this is about not registering, not about renaming), -#: so all five ranges resolve through :meth:`~pcapkit.corekit.enum. -#: EnumRegistry._unregistered_member` now and none of them mint. +#: hex-suffixed name unchanged -- PR #878 scoped the change this way: +#: preserve the existing name argument exactly, since this is about not +#: registering rather than about renaming -- so all five ranges resolve +#: through :meth:`~pcapkit.corekit.enum.EnumRegistry._unregistered_member` +#: now and none of them mint. EXPECTED_MISSING_NAMES = { 0x0000: 'Unspecified', # a defined member 0x0001: 'Routing_Information_Packet', # a defined member From 9bd738dd3a58e4da199fb0428afc02e8e24af96f Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 2 Oct 2026 09:12:30 -0400 Subject: [PATCH 6/9] test(corekit,const): narrow the blanket paraphrase, restoring quotations that cite correctly (#719) The last two commits paraphrased every disputed owner quotation away. That was right for one case and wrong for two: a quotation that exists nowhere has to be paraphrased, but a quotation that is real and was only cited to the wrong thread lost its audit trail for nothing, since the defect was the pointer, not the words. Per the owner's ruling, narrow the fix to match. Restored as quotations, correctly cited: - tests/corekit/test_sentinel_exports_unit.py (~L4-7) and tests/const/test_const_registry_protocol.py (~L1363): the sentinel export rule, split back into its two real sources instead of one spliced sentence -- #719's "we should ONLY export the objects ... and leave the types ... out", and #911's own "we only expose the final objects to users", with #911 noted as both executor and source. - tests/const/test_const_enum_no_mint.py (~L88, ~L1876, ~L2363) and tests/const/test_const_enum_builtin_parity.py (~L655): the #860 minting ruling, including its load-bearing first sentence ("I think we should not mint on get still actually") and the owner's own "entires" typo, marked [sic] rather than silently corrected. Left alone: the four #878 fabricated-quote sites in test_const_enum_no_mint.py, which cite no real thread and stay paraphrased, and the ABSENT privacy sentence, which is a correct paraphrase of a different ruling. Verified: ast.parse and reST markup clean on all four files; code token sequences (docstrings/comments stripped) identical before and after; each restored quotation substring-matches its source comment after whitespace/markup normalisation. tests/const: 299 OK. tests/ project/test_conventions_doc_claims: 38 OK, 1 skipped. --- tests/const/test_const_enum_builtin_parity.py | 12 +++++---- tests/const/test_const_enum_no_mint.py | 27 +++++++++++-------- tests/const/test_const_registry_protocol.py | 12 ++++++--- tests/corekit/test_sentinel_exports_unit.py | 10 ++++--- 4 files changed, 37 insertions(+), 24 deletions(-) diff --git a/tests/const/test_const_enum_builtin_parity.py b/tests/const/test_const_enum_builtin_parity.py index fc9500e3b0..6751cf226e 100644 --- a/tests/const/test_const_enum_builtin_parity.py +++ b/tests/const/test_const_enum_builtin_parity.py @@ -652,11 +652,13 @@ class ConstEnumRegisterFallbackTests(unittest.TestCase): transport subclasses) were, until GitHub issue #860 step 2, the last registries still living that divergence for an *unrecognised* value (every numeric registry had already lost it under #775/#847's ruling). - The owner's #860 ruling retired it for all of them: only IANA-registered - values are legitimate, so creating a new registry entry is - :meth:`register`'s job alone, and ``get()`` has no way to supply what - that would take. Stated for the three PR 1 converted, but the reasoning is - unconditional and PR 2 applies it to ``AppType`` identically. Concretely, + The owner's #860 ruling retired it for all of them: *"I think we should + not mint on* ``get`` *still actually. For all three, only IANA + registered ones are legit values and we need* ``register`` *to properly + create new entires [sic].* ``get`` *will not have sufficient + information to create new ones."* Stated for the three PR 1 converted, + but the reasoning is unconditional and PR 2 applies it to ``AppType`` + identically. Concretely, :class:`Command` needs :attr:`~pcapkit.const.ftp.command.Command.feat`/ :attr:`~pcapkit.const.ftp.command.Command.desc`/ :attr:`~pcapkit.const.ftp.command.Command.type`/ diff --git a/tests/const/test_const_enum_no_mint.py b/tests/const/test_const_enum_no_mint.py index 2d55ac94ac..36f99ffad4 100644 --- a/tests/const/test_const_enum_no_mint.py +++ b/tests/const/test_const_enum_no_mint.py @@ -85,9 +85,11 @@ initially left minting pending the owner's ruling (this measurement's own report flagged them as genuinely ambiguous under the criterion, since nothing about them is a manufactured placeholder). The owner's ruling on -#860 settled it: only IANA-registered values are legitimate, and creating a -new registry entry is :meth:`register`'s job alone -- nothing else has -enough information to supply one. That reasoning reaches ``get()`` as well as +#860 settled it: *"I think we should not mint on* ``get`` *still actually. +For all three, only IANA registered ones are legit values and we need* +``register`` *to properly create new entires [sic].* ``get`` *will not have +sufficient information to create new ones."* That reasoning reaches +``get()`` as well as ``_missing_`` -- :class:`Command` needs ``feat``/``desc``/``type``/``conf`` and :class:`Method` needs ``safe``/``idempotent``, neither of which a bare wire string carries -- so both classes' own ``get()`` (a second, independent @@ -1871,11 +1873,12 @@ class BespokeOpenVocabularyUnmintConvertedTests(unittest.TestCase): (``Unassigned_%d``, ``Unknown_%d``); each minted the literal, exact string it was asked to resolve, as its own name. That initially read as a case for keeping them minting (the label was never manufactured), but - the owner's ruling on #860 settled it the other way: only IANA-registered - values are legitimate, creating a new registry entry is - :meth:`register`'s job, and ``get()`` cannot supply what that would - take. Concretely: :class:`~pcapkit.const.ftp.command.Command` - needs ``feat``/``desc``/``type``/``conf`` and + the owner's ruling on #860 settled it the other way: *"I think we should + not mint on* ``get`` *still actually. For all three, only IANA + registered ones are legit values and we need* ``register`` *to properly + create new entires [sic].* ``get`` *will not have sufficient + information to create new ones."* Concretely: + :class:`~pcapkit.const.ftp.command.Command` needs ``feat``/``desc``/``type``/``conf`` and :class:`~pcapkit.const.http.method.Method` needs ``safe``/``idempotent``, neither of which a bare wire string carries, so minting used to register a permanently hollowed-out member for each. @@ -2360,9 +2363,11 @@ class AppTypeUnmintConvertedTests(unittest.TestCase): merely being imported, which does not exist here, since every one of these 766 spans mints only on an actual port lookup -- and the owner's ruling for the whole ``AppType`` family draws no distinction between a - real name and a placeholder: only IANA-registered values are - legitimate, creating a new registry entry is :meth:`register`'s job, - and ``get()`` lacks the information to do it. A lookup resolving + real name and a placeholder: *"I think we should not mint on* ``get`` + *still actually. For all three, only IANA registered ones are legit + values and we need* ``register`` *to properly create new entires + [sic].* ``get`` *will not have sufficient information to create new + ones."* A lookup resolving ``TCP(6010)`` after this PR therefore returns an *unregistered* ``x11`` member -- correct as a service name, but absent from ``__members__``/``_value2member_map_`` until someone calls diff --git a/tests/const/test_const_registry_protocol.py b/tests/const/test_const_registry_protocol.py index d3f7fa3d3f..cd14154ed7 100644 --- a/tests/const/test_const_registry_protocol.py +++ b/tests/const/test_const_registry_protocol.py @@ -1360,10 +1360,14 @@ def test_type_is_the_dedicated_sentinel_class_and_only_the_object_is_exported(se """GitHub issue #911 reversed half of what this used to assert. It read ``assertIn('NoDefaultType', enum_module.__all__)`` -- the type - *and* the object were exported. GitHub issue #719 settled that only the - object belongs in :attr:`__all__`, and #911 carried that out: the type - is out of :attr:`__all__` while staying importable by its dotted path, - which is what the last assertion here pins. + *and* the object were exported. GitHub issue #719 is where the owner + settled that: *"we should ONLY export the objects (like* ``NULL`` *) + to* ``__all__`` *, and leave the types (like* ``NullType`` *) out."* + Issue #911 carried that out -- and is also where he gave his own + "to users" framing for it: *"The general idea is that we only expose + the final objects to users."* The type is out of :attr:`__all__` + while staying importable by its dotted path, which is what the last + assertion here pins. """ import pcapkit.corekit.enum as enum_module diff --git a/tests/corekit/test_sentinel_exports_unit.py b/tests/corekit/test_sentinel_exports_unit.py index 64a14a53b6..1e0d3b72c4 100644 --- a/tests/corekit/test_sentinel_exports_unit.py +++ b/tests/corekit/test_sentinel_exports_unit.py @@ -1,10 +1,12 @@ # -*- coding: utf-8 -*- """GitHub issue #911: a sentinel exports its **object**, never its type. -GitHub issue #719 is where the owner settled this: a module's :attr:`__all__` -should list a sentinel's object (like ``NULL``), never its type (like -``NullType``). Issue #911 carried that ruling out. That is one rule with two -directions, and the tree on ``origin/main`` broke it in both: +GitHub issue #719 is where the owner settled this: *"we should ONLY export +the objects (like* ``NULL`` *) to* ``__all__`` *, and leave the types (like* +``NullType`` *) out."* Issue #911 carried that ruling out -- and is also +where he gave his own "to users" framing for the same rule: *"The general +idea is that we only expose the final objects to users."* That is one rule +with two directions, and the tree on ``origin/main`` broke it in both: * ``pcapkit.corekit.module.__all__`` was ``['NULL', 'NullType', 'ModuleDescriptor']`` -- the type is exported; From f24e030a0d9148861e7d54d21b493e4fcf0985d8 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 2 Oct 2026 10:27:14 -0400 Subject: [PATCH 7/9] test(corekit,const): convert restored quotations to statements with context, per #719 The previous commit restored eight verbatim quotations to fix a narrowing that had dropped their context. The owner has since ruled that neither form is right: a narrowed paraphrase without context does not help a reader who was not in the thread, but a verbatim quotation makes the docstring read as a discussion rather than documentation. - Sentinel export rule (corekit/test_sentinel_exports_unit.py, const/test_const_registry_protocol.py): state that a module's `__all__` lists a sentinel's object but deliberately leaves its type out, and why (the type is not part of the public surface), citing #719 as where it was settled and #911 as where the implementing work belongs. - #860 minting rule, four sites (const/test_const_enum_no_mint.py x3, const/test_const_enum_builtin_parity.py): state that `get()` must not mint and only `register()` creates a new entry, and why (only IANA-registered values are legitimate and `get()` lacks the information to construct one), citing #860. Each site is fitted to its own surrounding prose rather than one paragraph pasted four times. Drops the `[sic]` each quotation carried, since there is nothing left to reproduce. - Fixed two sentences left orphaned by the quotations' removal: an antecedent ("the three") that depended on the deleted quote's wording, and a sentence whose "get() as well as _missing_" had the emphasis backwards relative to the rule's own subject. The four PR #878 paraphrase sites in test_const_enum_no_mint.py were already in this third form and are unchanged. Verified: ast.parse on all four files; tokenize with comments and docstrings stripped shows an identical token sequence before/after (prose-only); tests/const (299) and tests/project/test_conventions_doc_claims.py (38, 1 skip) pass; tests/corekit (400, 5 failures, 16 skipped) matches the documented pre-existing sentinel-identity failures. --- tests/const/test_const_enum_builtin_parity.py | 13 ++++---- tests/const/test_const_enum_no_mint.py | 31 +++++++++---------- tests/const/test_const_registry_protocol.py | 15 ++++----- tests/corekit/test_sentinel_exports_unit.py | 14 +++++---- 4 files changed, 38 insertions(+), 35 deletions(-) diff --git a/tests/const/test_const_enum_builtin_parity.py b/tests/const/test_const_enum_builtin_parity.py index 6751cf226e..d973a1638c 100644 --- a/tests/const/test_const_enum_builtin_parity.py +++ b/tests/const/test_const_enum_builtin_parity.py @@ -652,12 +652,13 @@ class ConstEnumRegisterFallbackTests(unittest.TestCase): transport subclasses) were, until GitHub issue #860 step 2, the last registries still living that divergence for an *unrecognised* value (every numeric registry had already lost it under #775/#847's ruling). - The owner's #860 ruling retired it for all of them: *"I think we should - not mint on* ``get`` *still actually. For all three, only IANA - registered ones are legit values and we need* ``register`` *to properly - create new entires [sic].* ``get`` *will not have sufficient - information to create new ones."* Stated for the three PR 1 converted, - but the reasoning is unconditional and PR 2 applies it to ``AppType`` + GitHub issue #860 is where the owner retired it for all of them: + ``get()`` must not mint, because only ``register()`` may create a new + registry entry, and for these registries only IANA-registered values + are legitimate -- ``get()`` does not have enough information to build + one on its own. Stated for :class:`Method`, :class:`Command` and + :class:`FEATCode`, the three registries PR 1 converted, but the + reasoning is unconditional and PR 2 applies it to ``AppType`` identically. Concretely, :class:`Command` needs :attr:`~pcapkit.const.ftp.command.Command.feat`/ :attr:`~pcapkit.const.ftp.command.Command.desc`/ diff --git a/tests/const/test_const_enum_no_mint.py b/tests/const/test_const_enum_no_mint.py index 36f99ffad4..5108a20da7 100644 --- a/tests/const/test_const_enum_no_mint.py +++ b/tests/const/test_const_enum_no_mint.py @@ -84,12 +84,12 @@ :class:`~pcapkit.const.http.method.Method` -- 1 ``_missing_`` branch each, initially left minting pending the owner's ruling (this measurement's own report flagged them as genuinely ambiguous under the criterion, since -nothing about them is a manufactured placeholder). The owner's ruling on -#860 settled it: *"I think we should not mint on* ``get`` *still actually. -For all three, only IANA registered ones are legit values and we need* -``register`` *to properly create new entires [sic].* ``get`` *will not have -sufficient information to create new ones."* That reasoning reaches -``get()`` as well as +nothing about them is a manufactured placeholder). GitHub issue #860 is +where the owner settled it: ``get()`` must not mint, because only +``register()`` creates a new registry entry -- for these three registries +only IANA-registered values are legitimate, and ``get()`` does not have +enough information to construct one itself. That rule is about ``get()`` +in its own right, and it extends just as much to ``_missing_`` -- :class:`Command` needs ``feat``/``desc``/``type``/``conf`` and :class:`Method` needs ``safe``/``idempotent``, neither of which a bare wire string carries -- so both classes' own ``get()`` (a second, independent @@ -1873,11 +1873,10 @@ class BespokeOpenVocabularyUnmintConvertedTests(unittest.TestCase): (``Unassigned_%d``, ``Unknown_%d``); each minted the literal, exact string it was asked to resolve, as its own name. That initially read as a case for keeping them minting (the label was never manufactured), but - the owner's ruling on #860 settled it the other way: *"I think we should - not mint on* ``get`` *still actually. For all three, only IANA - registered ones are legit values and we need* ``register`` *to properly - create new entires [sic].* ``get`` *will not have sufficient - information to create new ones."* Concretely: + GitHub issue #860 settled it the other way: ``get()`` does not mint, + because only ``register()`` creates a new registry entry, and for these + registries only IANA-registered values are legitimate -- ``get()`` + simply does not have enough information to build a new one. Concretely: :class:`~pcapkit.const.ftp.command.Command` needs ``feat``/``desc``/``type``/``conf`` and :class:`~pcapkit.const.http.method.Method` needs ``safe``/``idempotent``, neither of which a bare wire string carries, so @@ -2363,11 +2362,11 @@ class AppTypeUnmintConvertedTests(unittest.TestCase): merely being imported, which does not exist here, since every one of these 766 spans mints only on an actual port lookup -- and the owner's ruling for the whole ``AppType`` family draws no distinction between a - real name and a placeholder: *"I think we should not mint on* ``get`` - *still actually. For all three, only IANA registered ones are legit - values and we need* ``register`` *to properly create new entires - [sic].* ``get`` *will not have sufficient information to create new - ones."* A lookup resolving + real name and a placeholder: GitHub issue #860 settled that ``get()`` + must not mint regardless of whether the value it would construct is a + genuine IANA-registered name or a manufactured placeholder, because + only ``register()`` may create a new entry and ``get()`` never has + enough information to build one itself. A lookup resolving ``TCP(6010)`` after this PR therefore returns an *unregistered* ``x11`` member -- correct as a service name, but absent from ``__members__``/``_value2member_map_`` until someone calls diff --git a/tests/const/test_const_registry_protocol.py b/tests/const/test_const_registry_protocol.py index cd14154ed7..dc6ae9c8f4 100644 --- a/tests/const/test_const_registry_protocol.py +++ b/tests/const/test_const_registry_protocol.py @@ -1361,13 +1361,14 @@ def test_type_is_the_dedicated_sentinel_class_and_only_the_object_is_exported(se It read ``assertIn('NoDefaultType', enum_module.__all__)`` -- the type *and* the object were exported. GitHub issue #719 is where the owner - settled that: *"we should ONLY export the objects (like* ``NULL`` *) - to* ``__all__`` *, and leave the types (like* ``NullType`` *) out."* - Issue #911 carried that out -- and is also where he gave his own - "to users" framing for it: *"The general idea is that we only expose - the final objects to users."* The type is out of :attr:`__all__` - while staying importable by its dotted path, which is what the last - assertion here pins. + settled the rule behind this: a module's ``__all__`` lists a + sentinel's object (such as ``NO_DEFAULT``), and deliberately leaves + its type (such as ``NoDefaultType``) out, because the type is not + part of the public surface -- exposing only the final object is what + keeps the published API to what a caller actually uses. GitHub issue + #911 is the issue that ruling's implementing work belongs to. The + type is out of :attr:`__all__` while staying importable by its + dotted path, which is what the last assertion here pins. """ import pcapkit.corekit.enum as enum_module diff --git a/tests/corekit/test_sentinel_exports_unit.py b/tests/corekit/test_sentinel_exports_unit.py index 1e0d3b72c4..bac9ae0f9f 100644 --- a/tests/corekit/test_sentinel_exports_unit.py +++ b/tests/corekit/test_sentinel_exports_unit.py @@ -1,12 +1,14 @@ # -*- coding: utf-8 -*- """GitHub issue #911: a sentinel exports its **object**, never its type. -GitHub issue #719 is where the owner settled this: *"we should ONLY export -the objects (like* ``NULL`` *) to* ``__all__`` *, and leave the types (like* -``NullType`` *) out."* Issue #911 carried that ruling out -- and is also -where he gave his own "to users" framing for the same rule: *"The general -idea is that we only expose the final objects to users."* That is one rule -with two directions, and the tree on ``origin/main`` broke it in both: +GitHub issue #719 is where the owner settled this rule: a module's +``__all__`` lists a sentinel's object (such as ``NULL``), and deliberately +leaves its type (such as ``NullType``) out. The reason is the context a +bare rule would not give: the types are not part of the public surface, so +exposing only the final objects is what keeps the published API to the +things a caller actually uses. GitHub issue #911 is the issue the +implementing work for that ruling belongs to. That is one rule with two +directions, and the tree on ``origin/main`` broke it in both: * ``pcapkit.corekit.module.__all__`` was ``['NULL', 'NullType', 'ModuleDescriptor']`` -- the type is exported; From d4d7702f14076c24ae6e587a40b3873a08188fb3 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 2 Oct 2026 10:50:56 -0400 Subject: [PATCH 8/9] test(corekit,const): state the remaining owner rulings in our own words, per #719 The four files still carried owner-attributed quotations beside the eight converted earlier, so each read half as documentation and half as a thread. - Replace each quoted ruling with a statement of the rule, the reason a reader needs, and the issue where it was given (#842, #864, #775, #860, #911, #719, #647, #808, #759, #857). - Rename the dangling "privacy ruling quoted below" reference to point at the statement that replaced the quotation. - Leave RFC text, code literals and ordinary prose untouched. Prose only: tokens with comments and docstrings stripped are identical before and after; tests/const 299 OK, tests/corekit unchanged (5 known). --- tests/const/test_const_enum_builtin_parity.py | 22 ++-- tests/const/test_const_enum_no_mint.py | 107 +++++++++--------- tests/const/test_const_registry_protocol.py | 60 ++++++---- tests/corekit/test_sentinel_exports_unit.py | 25 ++-- 4 files changed, 120 insertions(+), 94 deletions(-) diff --git a/tests/const/test_const_enum_builtin_parity.py b/tests/const/test_const_enum_builtin_parity.py index d973a1638c..5a97cb05f5 100644 --- a/tests/const/test_const_enum_builtin_parity.py +++ b/tests/const/test_const_enum_builtin_parity.py @@ -643,9 +643,9 @@ class ConstEnumRegisterFallbackTests(unittest.TestCase): """A once-sanctioned divergence, now retired for these three specific registries. - In the owner's words, the const enums used to mirror the built-in "with - one exception: they contain the missing then register fallback (mutable - enums)". :class:`~pcapkit.const.http.method.Method`, + The owner's design intent, recorded on GitHub issue #647, was for the const + enums to mirror the built-in enum's behaviour with one exception: a missing + value falls back to registering it (the mutable enums). :class:`~pcapkit.const.http.method.Method`, :class:`~pcapkit.const.ftp.command.Command`, :class:`~pcapkit.const.ftp.command.FEATCode` and :class:`~pcapkit.const.reg.apptype.apptype.AppType` (with its four @@ -850,10 +850,10 @@ def test_transport_protocol_can_no_longer_be_extended_at_runtime(self) -> None: alone: stock ``ad4805f5f`` still mints at ``max_val * 2``, so ``.get('quic')`` there returns ``16``, right after ``dccp``'s ``8``. This PR's own intermediate revision, not #808, is what switched an - unrecognised name to ``max_val + 1`` instead, minting ``9``. PR - #836's own inline comment on ``TransportProtocol.get`` removes the - registration entirely regardless of which scheme numbered it: "Do - not allow extension of TransportProtocol at all." Unlike + unrecognised name to ``max_val + 1`` instead, minting ``9``. The + review ruling behind #808 removes the registration from + ``TransportProtocol.get`` entirely, regardless of which scheme numbered + it: ``TransportProtocol`` is not to be extended at all. Unlike :class:`~pcapkit.const.ipv4.protection_authority.ProtectionAuthority` and :class:`~pcapkit.const.mh.cga_type.CGAType` below, ``TransportProtocol`` was never one of :data:`EXPECTED_TO_REGISTER` @@ -1001,10 +1001,10 @@ def test_self_check_of_the_percent_format_walk(self) -> None: def test_no_percent_formatting_survives_in_the_issue_804_pair(self) -> None: """Neither const module nor either generator still formats with ``%``. - The maintainer's convention from GitHub issue #783: *"id like to keep - f-string convention across the library. only use % substitution when - inevitable."* A ``__repr__`` is not an inevitable case, and neither is - a ``wrap_comment`` argument. + The house convention, set in review of the fix for GitHub issue #759: + f-strings across the whole library, with ``%`` substitution only where + it is inevitable. A ``__repr__`` is not an inevitable case, and neither + is a ``wrap_comment`` argument. Covers the vendor modules as well as the const ones because the tree is generated: a conversion that is not in the template is reverted by diff --git a/tests/const/test_const_enum_no_mint.py b/tests/const/test_const_enum_no_mint.py index 5108a20da7..4bfd53f7ab 100644 --- a/tests/const/test_const_enum_no_mint.py +++ b/tests/const/test_const_enum_no_mint.py @@ -1,10 +1,10 @@ # -*- coding: utf-8 -*- """Regression tests for GitHub issue #775's tier 1: the miss path must not mint. -The maintainer's ruling on #775, verbatim: *"so that we dont create registered -enums out of unrecognised/unregistered values, unless user/caller explicitly -created them"* -- and lookup never counts as asking for a name, only the new -:meth:`register` classmethod does. Before this change, both ``get()``'s string +The ruling on #775 is that no registry creates a registered enum out of an +unrecognised or unregistered value, however legitimate but unbounded the values +are, unless the user or caller explicitly created it -- and lookup never counts as +asking for a name, only the new :meth:`register` classmethod does. Before this change, both ``get()``'s string path and ``_missing_``'s bounded-but-unassigned range branch called :func:`aenum.extend_enum`, permanently growing the registry for a value nobody asked to be named. Measured on this tree before the fix: @@ -24,10 +24,13 @@ Tier 1 fixed the *mechanism* (a lookup should never mint); it did not decide, registry by registry, which of the remaining ``_missing_`` bodies mint -something worth keeping. That is tier 2; the owner's mint/unmint criterion, -settled on GitHub pull request #847 and confirmed on #775, is verbatim: *"a -final concrete assigned name -> mint; a notation for the readers -> unmint"*. -Applied registry by registry to every ``_missing_`` that still called +something worth keeping. That is tier 2; the owner's mint/unmint criterion was +settled while fixing #841 and confirmed on #775 as the core of the ruling: a name +is minted when it is the final concrete name an assignment gave, and left +unminted when it is only a notation for the readers. A dynamically or statically +assigned range is as unspecified as any other -- it gets concrete names when +something assigns them -- so its placeholder is notation, not a name worth +registering. Applied registry by registry to every ``_missing_`` that still called :func:`~aenum.extend_enum` -- measured at exactly 89 modules by an AST walk over ``pcapkit/const/*.py`` (not the "~92" an earlier pass in this programme estimated) -- the ruling converted 82 @@ -113,33 +116,32 @@ name a real (if IANA-assigned to a whole span rather than declared individually) service rather than a placeholder. -Untouched, per the owner's ruling settling the ``needs: decision`` this PR -re-opened: :class:`~pcapkit.const.ftp.command.CommandType` keeps ``IntFlag``, -verbatim: *"okay, let's keep IntFlag if that's how RFC/IANA data is -constructed (`|` may exist on the CSV data)"* -- measured, 2 occurrences in +Untouched, per the owner's ruling on #860 settling the ``needs: decision`` that +issue re-opened: :class:`~pcapkit.const.ftp.command.CommandType` keeps +``IntFlag``, because that is how the RFC/IANA data is constructed and ``|`` may +appear in the CSV -- measured, 2 occurrences in the generated data join two kinds with ``/`` and would break under a plain ``IntEnum``; see :class:`BespokeOpenVocabularyUnmintConvertedTests`'s own ``test_commandtype_is_untouched``. :class:`~pcapkit.const.reg.apptype.apptype.TransportProtocol`, by contrast, *did* change -- on a different ruling than CommandType's, not the same one. -GitHub PR #836 is what first retired ``|``-composite decoding, verbatim: -*"since it's no longer a Flag, `|` joined values are no longer parsed and -accepted, we will treat it as a whole, instead of splitting"* -- and it was -this issue, #860, that later drew the further consequence once nothing -decoded a composite any more: *"2-power on TransportProtocol is mainly for -the consideration of previously `tcp | udp`-alike code. But we dont accept -this kind of piping anymore in the new logic so I think `auto()` is the -expected behaviour."* So it now numbers its five members sequentially from +GitHub issue #808 is what first retired ``|``-composite decoding: once +``TransportProtocol`` is no longer a ``Flag``, a ``|``-joined value is not parsed +and accepted but treated as a whole, instead of being split. GitHub issue #860 +later drew the further consequence once nothing decoded a composite any more: +the power-of-two spacing existed only for the ``tcp | udp``-style code that +composition served, and since the new logic does not accept that piping, ``auto()`` +is the expected numbering. So it now numbers its five members sequentially from 0 -- ``undefined`` a direct, explicit ``0``, the rest continuing from it via ``auto()`` with no ``_start_`` needed, per the owner's own further ruling settling the declaration shape -- rather than by the power-of-two -spacing that composition used to need; per the owner's own follow-up, -*"no need to add test around -that"* for the one behavioural consequence -- and it is a real consequence, +spacing that composition used to need. The owner also ruled, on #860, that no +test should pin the one behavioural consequence, since it is an obsolete path +left by an accepted breaking change -- and it is a real consequence, not a refusal: a hand-composed ``tcp | udp`` (or a bare, uncomposed ``3``) now silently resolves as ``sctp``'s own value through :meth:`AppType._dispatch`, where it used to name no registry at all and -raise. None of the tests below pin that, per the same instruction. +raise. None of the tests below pin that, per the same ruling. GitHub issue #775's final round closes the two mixed registries themselves: every one of :class:`~pcapkit.const.reg.ethertype.EtherType`'s 52 still- @@ -798,10 +800,10 @@ def test_unresolvable_string_key_with_default_in_unassigned_range(self) -> None: pseudo-member the int path does -- it simply does not resolve, and the original key's own error propagates instead. Before #864 this resolved via ``cls(default)`` -> ``_missing_`` to an unregistered - pseudo-member; that is exactly the cost the owner's ruling accepts - ("a default naming a value with no registered member stops - resolving, on every registry... today such a default returns an - unregistered member via _missing_").""" + pseudo-member; that is exactly the cost the owner accepted on #864 in + choosing a value-only lookup: a default naming a value with no + registered member stops resolving, on every registry, where it used + to return an unregistered member via ``_missing_``.""" from pcapkit.const.arp.hardware import Hardware before = len(Hardware.__members__) @@ -1206,10 +1208,13 @@ def _is_hex_suffixed_unregistered_name(name_arg: 'Optional[ast.expr]') -> bool: class UnregisteredMemberNameIsBareTests(unittest.TestCase): - """GitHub pull request #838's Q1 follow-up, the maintainer's ruling verbatim: - *"Q1 - bare it is."* Asked whether the non-minting path should honour the - registry's own ``unassigned``/``reserved`` name directly or keep appending - the numeric value, he chose the bare name -- safe precisely because a + """The naming rule settled under GitHub issue #775's tier 1: a non-minting + pseudo-member carries the registry's own ``unassigned``/``reserved`` name + bare, with no numeric value appended. Minted members keep their number, + because there it is part of the real name (``Motorola_0x8705`` names one + EtherType inside Motorola's block). Asked whether the non-minting path should + honour the registry's own name directly or keep appending the numeric value, + the owner chose the bare name -- safe precisely because a pseudo-member built by :meth:`_unregistered_member` never enters ``__members__``/``_member_map_``/``_value2member_map_``, so two same-named pseudo-members (e.g. ``Chunk._unregistered_member(20, 'Unassigned')`` and @@ -1922,11 +1927,10 @@ def test_featcode_import_mints_nothing(self) -> None: so every :class:`Command` row references one by plain attribute access and nothing is minted merely by importing the module. - Deliberately not a literal member count: the owner's own words, - *"this might break when IANA updated their list. i dont like this - guard on the test cases"* -- a regeneration that correctly picks up - a newly-registered FEAT code would fail a hardcoded number for being - *right*. The invariant that survives a table update instead: every + Deliberately not a literal member count: the owner ruled against + that guard on #860, because a hardcoded number breaks whenever IANA + updates its list -- a regeneration that correctly picks up + a newly-registered FEAT code would fail it for being *right*. The invariant that survives a table update instead: every name in ``__members__`` is a real declaration in the generated source, not something built by a call at import time. A regeneration moves declarations and members together; only an @@ -2142,10 +2146,10 @@ def test_all_three_carry_the_registry_protocol(self) -> None: self.assertTrue(callable(getattr(cls, 'register_alias', None))) def test_commandtype_is_untouched(self) -> None: - """The owner's ruling also floated ``CommandType`` -> ``IntEnum``. - Re-opened as `needs: decision` on #860 and settled the other way, - verbatim: *"okay, let's keep IntFlag if that's how RFC/IANA data is - constructed (`|` may exist on the CSV data)."* Measured: 2 occurrences + """``CommandType`` -> ``IntEnum`` was floated too. + Re-opened as `needs: decision` on #860 and settled the other way: it + keeps ``IntFlag``, because that is how the RFC/IANA data is constructed + and ``|`` may appear in the CSV. Measured: 2 occurrences in the generated data join two kinds with ``/``, e.g. access *and* parameter, which a plain ``IntEnum`` cannot represent. Pinned here so a future change is noticed as a scope change rather than folded @@ -2645,15 +2649,14 @@ def test_apptype_unrecognised_proto_is_still_refused(self) -> None: class TransportProtocolAutoTests(unittest.TestCase): """:class:`~pcapkit.const.reg.apptype.apptype.TransportProtocol`'s - ``auto()`` conversion, #860 step 2 PR 2's other change. The owner's - ruling, verbatim: *"since we do not allow `|` anymore, using `auto()` - instead of 2-power is the right move (breaking change but accepted)."* - And, on the one behavioural consequence -- a hand-composed + ``auto()`` conversion, #860 step 2 PR 2's other change. The owner ruled on + #860 that, since ``|`` composition is no longer allowed, ``auto()`` in place + of power-of-two spacing is the right move -- a breaking change, but an + accepted one. On the one behavioural consequence -- a hand-composed ``tcp | udp`` now equalling ``sctp`` numerically (``1 | 2 == 3``), where it - used to name no member at all and be refused as a whole -- the explicit - follow-up settling that no test should pin it: *"No need to add test - around that honestly. This is an obsoleted path from a breaking - change."* So this class deliberately covers only the values and the + used to name no member at all and be refused as a whole -- the same thread + settled that no test should pin it: the path is obsolete after the breaking + change, and a test would preserve it as though it still mattered. So this class deliberately covers only the values and the stale comment's removal, not the composition consequence: no test here calls ``_dispatch`` with a composed value at all, unlike :meth:`AppTypeUnmintConvertedTests. @@ -2696,9 +2699,9 @@ def test_get_still_refuses_an_unrecognised_name(self) -> None: TransportProtocol.get('not-a-real-transport') def test_stale_power_of_two_comment_is_gone(self) -> None: - """The owner's instruction, verbatim: the in-code comment claiming - the values "must keep" power-of-two spacing contradicted GitHub - PR #836's own ruling once nothing decomposed a composite any + """Per GitHub issue #860: the in-code comment claiming + the values "must keep" power-of-two spacing contradicted the #808 + ruling that nothing composes a ``TransportProtocol`` any more, and is deleted rather than merely superseded -- checked against the generated source itself, not the docstring here, so a regeneration that reintroduces it fails this test.""" diff --git a/tests/const/test_const_registry_protocol.py b/tests/const/test_const_registry_protocol.py index dc6ae9c8f4..5f0d75c3d4 100644 --- a/tests/const/test_const_registry_protocol.py +++ b/tests/const/test_const_registry_protocol.py @@ -7,11 +7,11 @@ their own, because each of those carries a hand-copied ``get()`` -- and none of them carries ``register``, ``register_alias`` or ``get_all`` at all. -The maintainer's ruling on #842, verbatim: *"to finalise the abstraction idea, -get/get_all/register/register_alias should always exist on the const enums - so -they're to be moved to the base class. And AppType's sub-base class will do its -necessary overrides and dispatching logic; AppType subclasses will have their -necessary overrides again pertaining their different contracts."* +The design ruling on #842 finalises the abstraction: ``get``, ``get_all``, +``register`` and ``register_alias`` should always exist on the const enums, so they +move to the base class and every registry answers the same four calls. The +``AppType`` sub-base class keeps its own overrides and dispatching logic, and its +subclasses override further where their contracts differ. :class:`~pcapkit.corekit.enum.EnumRegistry` is tier one of that hierarchy. This module pins both halves of the claim: that the generated registries in this batch @@ -322,7 +322,10 @@ def test_tcp_flags_gains_the_protocol_it_never_had(self) -> None: class GetContractTests(unittest.TestCase): - """*"get is a shortcut for ``[]`` operation and returns the canonical enum."*""" + """``get`` is a shortcut for the ``[]`` operation and returns the canonical enum. + + The contract set on #842. + """ def setUp(self) -> None: snapshot = snapshot_modules(ISOLATED_PREFIXES) @@ -397,7 +400,10 @@ def test_get_never_mints_on_any_converted_registry(self) -> None: class GetAllContractTests(unittest.TestCase): - """*"get_all returns all matching enums."*""" + """``get_all`` returns every matching enum, not only the canonical one. + + The contract set on #842. + """ def setUp(self) -> None: snapshot = snapshot_modules(ISOLATED_PREFIXES) @@ -434,7 +440,11 @@ def test_get_all_propagates_a_miss(self) -> None: class RegisterContractTests(unittest.TestCase): - """*"register mints new enum to the class at runtime with specified names."*""" + """``register`` mints a new enum on the class at runtime under the names it is given. + + The contract set on #842: the caller supplies the names, so nothing has to guess + one blindly. + """ def setUp(self) -> None: snapshot = snapshot_modules(ISOLATED_PREFIXES) @@ -568,10 +578,13 @@ def test_register_alias_still_aliases_after_the_value_guard(self) -> None: class RegisterAliasContractTests(unittest.TestCase): - """*"register_alias(es) adds additional alias(es) to a given enum's mapping."* + """``register_alias`` adds additional alias(es) to a given enum's mapping. - And, on whether an enum must be given: *"actually i think it should always be - for an existing member"*. + The contract set on #842, which also ruled that an alias is always for an + existing member, unless ``AppType`` (and the concrete enums) must call it on a + member that does not exist yet. One method name then carries one convention + across the registries, and for the plain registries an alias is a caller-opt-in + name that IANA does not record. """ @@ -1499,13 +1512,16 @@ class GetDefaultNoMintTests(unittest.TestCase): and the value branch's -- reached ``_missing_`` for a ``default`` that fell inside a still-minting registry's own range, growing the registry as a side effect of resolving ``default`` rather than ``key``. The - owner's ruling, verbatim: *"Take (b). Only register can mint. get should - not mint unless it falls through the ``_missing_``'s minted ranges."*, - and on the implementation, choosing option 1 of three: *"I think 1 is - correct mechanism we'd like."* -- ``default`` now resolves through a - plain ``_value2member_map_`` lookup only, so it cannot mint by - construction, while ``key`` resolution -- and whatever it lets - ``_missing_`` do -- is deliberately unchanged. + owner ruled on #864 that only ``register`` may mint: ``get`` mints only + as the natural outcome of ``key`` falling through one of ``_missing_``'s + minted ranges, never because a fallback happened to resolve. Of the + three mechanisms offered, the owner chose the one that cannot mint by + construction -- ``default`` now resolves through a plain + ``_value2member_map_`` lookup only, with no snapshot-and-undo and no + per-class marker to keep in step with future crawlers -- at the price of a + ``default`` in a declared-but-unassigned range no longer resolving. + ``key`` resolution -- and whatever it lets ``_missing_`` do -- is + deliberately unchanged. """ def setUp(self) -> None: @@ -1551,10 +1567,10 @@ def test_the_issues_own_case_no_longer_mints_and_raises_instead(self) -> None: def test_key_path_minting_is_preserved(self) -> None: """Originally a no-change guard: ``EtherType.get(0x0888)`` -- no ``default`` at all -- used to mint ``Xyplex_0x0888`` through its own - ``_missing_``, back when ruling one's mint/unmint criterion - (*"get should not mint unless it falls through the _missing_'s - minted ranges"*) still classed ``Xyplex`` as a kept-minting, - real-attributed-name range. GitHub issue #775's final round converts + ``_missing_``, back when #775's original mint/unmint criterion still + classed ``Xyplex`` as a kept-minting, real-attributed-name range -- + the one kind of mint #864 ruled ``get`` may still produce, by falling + through ``_missing_``'s minted ranges. GitHub issue #775's final round converts that range (and every other one still minting on ``EtherType``/ :class:`~pcapkit.const.ipx.socket.Socket`) to :meth:`~pcapkit.corekit. enum.EnumRegistry._unregistered_member`, preserving the hex-suffixed diff --git a/tests/corekit/test_sentinel_exports_unit.py b/tests/corekit/test_sentinel_exports_unit.py index bac9ae0f9f..f78aaedb5a 100644 --- a/tests/corekit/test_sentinel_exports_unit.py +++ b/tests/corekit/test_sentinel_exports_unit.py @@ -37,12 +37,15 @@ pins the count and the doc together, so the next sentinel cannot be added to one without the other. ``ABSENT`` is private and stays out of :attr:`__all__` in both directions -- not under the export ruling, which argues the opposite for an -object like this one, but under the privacy ruling quoted below, which treats a +object like this one, but under the privacy ruling stated below, which treats a sentinel documented as private as having no place in :attr:`__all__` at all. A follow-up to this same issue moved all four *definitions* into -:mod:`pcapkit.corekit.sentinels`, per the owner's later ruling -- *"Okay one module -for all four it is."* Every assertion above still holds unchanged, since it is about +:mod:`pcapkit.corekit.sentinels`: the owner chose one shared module over one module +per sentinel, on #911. The shared module is where the ``Type`` naming +convention and the rules for when to add ``__bool__``, ``__copy__`` or ``__reduce__`` +can sit together, which four near-empty files would scatter. +Every assertion above still holds unchanged, since it is about each original module's ``__all__``, which the re-export shims left untouched; what changed is only :attr:`type.__module__` for the four types, which :meth:`SentinelExportTests.test_every_sentinel_type_is_still_importable_by_name` now @@ -53,9 +56,9 @@ ``NoValue`` to ``NO_VALUE`` and ``_Absent`` to ``ABSENT``, the latter also dropping its leading underscore -- so every instance name agrees on one casing. Privacy for what is now ``ABSENT`` stopped being signalled by the name at all and became -documentation-only, per the owner's ruling on GitHub issue #719, verbatim: -*"we can change* ``_ABSENT`` *to* ``ABSENT`` *just document it as private -type/class in the documentation and not for public use is enough."* +documentation-only. GitHub issue #719 is where the owner ruled that the underscore +can go, because documenting the object and its type as private, and not for public +use, is enough on its own to carry the privacy. :meth:`SentinelExportTests.test_the_private_sentinel_is_exported_neither_way` is what now pins that privacy under the new name, since the mechanical underscore signal it used to double-check is gone. @@ -126,8 +129,8 @@ #: Repository root, for the two tests that read a file rather than import it. ROOT = pathlib.Path(__file__).resolve().parents[2] -#: Where all four sentinels are now *defined*, since GitHub issue #911's housing -#: move -- *"Okay one module for all four it is."* Each entry in :data:`SENTINELS` +#: Where all four sentinels are now *defined*: the single shared module GitHub +#: issue #911 chose over one module per sentinel. Each entry in :data:`SENTINELS` #: below used to name a different module here (``pcapkit.corekit.module``, #: ``pcapkit.corekit.fields.field``, ``pcapkit.corekit.enum`` and #: ``pcapkit.protocols.protocol`` respectively); all four now report this one. @@ -324,7 +327,11 @@ class SentinelPopulationTests(unittest.TestCase): """There are four, they follow the naming rule, and the docs say so.""" def test_every_sentinel_follows_the_naming_convention(self) -> 'None': - """*"Keep the sentinel object's type class naming as* ``Type``*."*""" + """A sentinel object's type class is named ``Type``. + + The house convention set under GitHub issue #857, kept uniform so that a + future maintainer adding a sentinel can derive the type's name mechanically. + """ for name, _, type_ in SENTINELS: with self.subTest(sentinel=name): self.assertEqual(type_.__name__, _expected_type_name(name)) From 9d048d2d48667026848b5b04ce20fb92cfefa50e Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 2 Oct 2026 11:18:48 -0400 Subject: [PATCH 9/9] test(const): restore ruling citations to the pull requests that carry them, per #719 tests/ is exempt from the issue-citation rule (documentation.rst, ruled on #719): the fact cited lives in the pull request, not the issue. - PR #836 restored for the TransportProtocol-extension refusal, the |-composite decoding retirement, and the stale-comment deletion; the rulings are not on #808 at all. - PR #783 for the f-string convention; PR #847 for the mint criterion. - The de-quotation stands: wording stays as statements, no quotation marks. --- tests/const/test_const_enum_builtin_parity.py | 4 ++-- tests/const/test_const_enum_no_mint.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/const/test_const_enum_builtin_parity.py b/tests/const/test_const_enum_builtin_parity.py index 5a97cb05f5..5f4fe846be 100644 --- a/tests/const/test_const_enum_builtin_parity.py +++ b/tests/const/test_const_enum_builtin_parity.py @@ -851,7 +851,7 @@ def test_transport_protocol_can_no_longer_be_extended_at_runtime(self) -> None: ``.get('quic')`` there returns ``16``, right after ``dccp``'s ``8``. This PR's own intermediate revision, not #808, is what switched an unrecognised name to ``max_val + 1`` instead, minting ``9``. The - review ruling behind #808 removes the registration from + review ruling on PR #836 removes the registration from ``TransportProtocol.get`` entirely, regardless of which scheme numbered it: ``TransportProtocol`` is not to be extended at all. Unlike :class:`~pcapkit.const.ipv4.protection_authority.ProtectionAuthority` @@ -1001,7 +1001,7 @@ def test_self_check_of_the_percent_format_walk(self) -> None: def test_no_percent_formatting_survives_in_the_issue_804_pair(self) -> None: """Neither const module nor either generator still formats with ``%``. - The house convention, set in review of the fix for GitHub issue #759: + The house convention, set in review on PR #783: f-strings across the whole library, with ``%`` substitution only where it is inevitable. A ``__repr__`` is not an inevitable case, and neither is a ``wrap_comment`` argument. diff --git a/tests/const/test_const_enum_no_mint.py b/tests/const/test_const_enum_no_mint.py index 4bfd53f7ab..81409d4c81 100644 --- a/tests/const/test_const_enum_no_mint.py +++ b/tests/const/test_const_enum_no_mint.py @@ -25,7 +25,7 @@ Tier 1 fixed the *mechanism* (a lookup should never mint); it did not decide, registry by registry, which of the remaining ``_missing_`` bodies mint something worth keeping. That is tier 2; the owner's mint/unmint criterion was -settled while fixing #841 and confirmed on #775 as the core of the ruling: a name +settled on PR #847 and confirmed on #775 as the core of the ruling: a name is minted when it is the final concrete name an assignment gave, and left unminted when it is only a notation for the readers. A dynamically or statically assigned range is as unspecified as any other -- it gets concrete names when @@ -125,7 +125,7 @@ ``test_commandtype_is_untouched``. :class:`~pcapkit.const.reg.apptype.apptype.TransportProtocol`, by contrast, *did* change -- on a different ruling than CommandType's, not the same one. -GitHub issue #808 is what first retired ``|``-composite decoding: once +GitHub PR #836 is what first retired ``|``-composite decoding: once ``TransportProtocol`` is no longer a ``Flag``, a ``|``-joined value is not parsed and accepted but treated as a whole, instead of being split. GitHub issue #860 later drew the further consequence once nothing decoded a composite any more: @@ -2700,7 +2700,7 @@ def test_get_still_refuses_an_unrecognised_name(self) -> None: def test_stale_power_of_two_comment_is_gone(self) -> None: """Per GitHub issue #860: the in-code comment claiming - the values "must keep" power-of-two spacing contradicted the #808 + the values "must keep" power-of-two spacing contradicted the PR #836 ruling that nothing composes a ``TransportProtocol`` any more, and is deleted rather than merely superseded -- checked against the generated source itself, not the docstring here, so a