From 3f64ab57b8a600e26bbb269d788d73091983bb3f Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 2 Oct 2026 13:20:32 -0400 Subject: [PATCH 1/4] docs(corekit): state the sentinel rulings instead of quoting them Part of #987. Five quotations across two files become statements, and one citation is corrected from authority to provenance. sentinels.py's comment above ABSENT read "per GitHub issue #937" for the fact that ABSENT is private by documentation rather than by underscore. That is a what-changed statement, not a ruling attribution: #937 is the SCREAMING_SNAKE rename, and its own body attributes the ruling to #719, which carries it. Now written with an action verb naming the issue the change belongs to. - Module docstring: the one-shared-module choice on #911 stated rather than quoted, naming what it was chosen over. - NoDefaultType: the dedicated-class ruling and the unsettled-naming remark, both given in review of the work for #857, stated as what they settled. - AbsentType: the rename ruling on #719 stated, including that documenting ABSENT as private replaces the underscore. - tests/corekit: two "I prefer (2) directly" quotations become statements. The pull-request citations stay, since tests/ is exempt from the issue-citation rule per docs/source/contributing/conventions/documentation.rst:203-210, and the wording matches what landed for the same ruling in #991. Three other quoted spans are left alone deliberately: two are documentation section titles and one is a caveat the docstring makes about the code, none of them a maintainer's words. Prose only: token sequences identical with strings masked in both files, the AST with docstrings blanked compares equal, and maximum line length is unchanged at 98 and 121. --- pcapkit/corekit/sentinels.py | 23 ++++++++++--------- .../test_enum_lookup_reparent_930_unit.py | 18 ++++++++------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/pcapkit/corekit/sentinels.py b/pcapkit/corekit/sentinels.py index 9b046038d..5a8505e64 100644 --- a/pcapkit/corekit/sentinels.py +++ b/pcapkit/corekit/sentinels.py @@ -17,8 +17,8 @@ :class:`NoValueType` in :mod:`pcapkit.corekit.fields.field`, :class:`NoDefaultType` in :mod:`pcapkit.corekit.enum` and :class:`AbsentType` in :mod:`pcapkit.protocols.protocol`. The owner's ruling -on GitHub issue #911, verbatim -- *"Okay one module for all four it is."* -- -moves the four *definitions* here; each original module keeps a three-line +on GitHub issue #911, choosing one shared module over one module per +sentinel, moves the four *definitions* here; each original module keeps a three-line re-export so that no existing ``from import `` breaks, including the ``if TYPE_CHECKING:``-only imports of the *types* that :mod:`pcapkit.foundation.registry.foundation`, @@ -239,8 +239,8 @@ class NoDefaultType: :meth:`EnumLookup.get `. 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 + review of the work for #857, which asked for a dedicated class that + follows 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 @@ -254,8 +254,9 @@ class NoDefaultType: use ``Type``. At the time, the *instance*'s own name was not 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: + (``CapWords``) disagreed, and that the choice should follow what each + sentinel is needed for rather than a settled rule. 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 @@ -456,9 +457,9 @@ 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 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 + on GitHub issue #719 accepted that rename, and held that documenting + ``ABSENT`` as a private type and class, not for public use, is enough + to replace the underscore. 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 @@ -486,6 +487,6 @@ def __repr__(self) -> 'str': #: `. Never leaves #: :mod:`pcapkit.protocols.protocol`, which keeps a private re-export of it #: for exactly that one read. Private by convention and documentation only, -#: not by a leading underscore -- see :class:`AbsentType`'s own docstring for -#: why, per GitHub issue #937. +#: not by a leading underscore, which GitHub issue #937 dropped -- see +#: :class:`AbsentType`'s own docstring for why. ABSENT = AbsentType() diff --git a/tests/corekit/test_enum_lookup_reparent_930_unit.py b/tests/corekit/test_enum_lookup_reparent_930_unit.py index 745408d00..f6d54a5c5 100644 --- a/tests/corekit/test_enum_lookup_reparent_930_unit.py +++ b/tests/corekit/test_enum_lookup_reparent_930_unit.py @@ -199,10 +199,10 @@ def test_esp_status(self) -> None: def test_fast_binding_acknowledgment_status(self) -> None: """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.""" + outright (the owner preferred deleting it to widening it to accept + ``default``), 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 from pcapkit.protocols.internet.mh import FastBindingAcknowledgmentStatus @@ -450,10 +450,12 @@ 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, 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` + final ruling, given on GitHub pull request #940, went the other way: an + earlier lean on GitHub issue #935 had favoured widening, but on + reviewing that attempt the owner preferred deleting both overrides + outright. 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. From 063157f817887ebd0efb79e84c33e4a7617fe246 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 2 Oct 2026 13:34:58 -0400 Subject: [PATCH 2/4] docs(tests): paraphrase the six rulings this file still quoted The cross-review on #992 found six maintainer quotations surviving in tests/corekit/test_enum_lookup_reparent_930_unit.py, one pair inside the very docstring whose second paragraph the first pass had already converted -- so a single __doc__ showed both forms fifteen lines apart. The first pass implemented #987's remainder list rather than measuring. That list said two "I prefer (2) directly" quotations; grep over main gives three, at lines 33, 202 and 454, and only the last two were converted. The issue body is corrected, and the scan this time sets no minimum span length -- one of the six was two characters long and a length filter is what hid it. - The #935 lean and the two #940 quotations at :29-33 become one statement of what each settled, keeping the pull-request citation. - #933's reversal is paraphrased differently at the two sites that draw on it, because they make different points: the module docstring takes what the ruling settled, while the class docstring takes why an earlier revision of this file had pinned the opposite. - The prose said the follow-up came four minutes after the first answer. The comments are 21:32:21Z and 21:37:17Z, so 4m56s; now "a few minutes later", which is what the evidence carries. The pull-request citations stay. tests/ is exempt from the name-the-issue rule per docs/source/contributing/conventions/documentation.rst:203, but that exemption does not reach the no-verbatim rule at :184, which has no carve-out. Prose only: token sequences identical with strings masked, both differing string tokens are docstrings, the AST with docstrings blanked compares equal, and maximum line length stays 121. A re-scan finds zero maintainer quotations. --- .../test_enum_lookup_reparent_930_unit.py | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/tests/corekit/test_enum_lookup_reparent_930_unit.py b/tests/corekit/test_enum_lookup_reparent_930_unit.py index f6d54a5c5..06a4388c3 100644 --- a/tests/corekit/test_enum_lookup_reparent_930_unit.py +++ b/tests/corekit/test_enum_lookup_reparent_930_unit.py @@ -26,11 +26,12 @@ argument raised :exc:`TypeError` instead of resolving through the base's own fallback, and ``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 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. +that on the owner's lean toward its first option -- widen both signatures to accept +``default`` and delete the suppression. On GitHub pull request #940, which was +implementing that widening, the owner then asked why the two overrides had to exist at +all rather than fall back to the base class's, and after the measurements below showed +them redundant, ruled for the second of the two options laid out there: 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 @@ -62,8 +63,9 @@ itself raised **loud**: both overrides used to log once at :data:`logging.CRITICAL` and set :data:`sys.tracebacklimit` to ``0`` process-wide on a name miss, unlike the base's own quiet raise. GitHub issue #930 converged both onto the base's quiet shape instead -- a real -behaviour change, not merely a re-parent -- settled on GitHub issue #933's follow-up ruling, -verbatim: *"Oh wait. I meant, they should follow house convention and not to be loud."* +behaviour change, not merely a re-parent -- settled on GitHub issue #933, where the owner +reversed an earlier answer: the two overrides should follow the library's house convention +for a name miss, which is the base's quiet raise, rather than stay loud as a special case. :class:`InheritedQuietnessTests` (renamed from ``KeptOverrideQuietnessTests`` once GitHub issue #935 deleted the overrides that name described) pins that the quiet shape survived the deletion too -- purely inherited now, rather than reconciled by hand on each class. @@ -464,12 +466,12 @@ class was ``KeptOverrideQuietnessTests`` while both classes still :data:`sys.tracebacklimit` to ``0`` process-wide on a name miss, unlike the base's own quiet raise. GitHub issue #930 converged both onto that quiet shape instead, settled on GitHub issue #933's follow-up ruling, - verbatim: *"Oh wait. I meant, they should follow house convention and - not to be loud."* (An earlier message on the same issue said the - opposite -- plain *"No."* -- and an earlier revision of this file - briefly pinned loud as the settled answer on the strength of that - message; the follow-up four minutes later superseded it, and what - follows is the corrected version.) + in which the owner clarified that the two should follow house + convention and not be loud. (The owner's first answer on the same + issue, a flat refusal, was read as keeping them loud, and an earlier + revision of this file briefly pinned loud as the settled answer on + the strength of that reading; the clarification a few minutes later + superseded it, and what follows is the corrected version.) The first two methods are pinned quiet, and for two different reasons against the tree reverted to before #930. From 9a1779e951b50330ce680138b8b78c890381f36d Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 2 Oct 2026 13:46:25 -0400 Subject: [PATCH 3/4] docs(tests): say what the #933 answer was, not how it was read Round 2's review found two accuracy defects in the paraphrases this pull request had just written, and they pull in opposite directions from the one this issue usually catches. The class docstring said the owner's first answer "was read as keeping them loud", and that an earlier revision pinned loud "on the strength of that reading". That dissolves a fact into a perception and puts the error on the reader. #933's body pre-labels its own option 2 as a refusal -- a get_all miss being a genuine failure the caller should see loudly -- so the bare answer selected a numbered option that already meant keep-loud. There was no other reading available. - The first answer is now stated as a bare refusal selecting the issue's own numbered option, with the file's earlier revision pinning loud on that basis. - The two sites described the same event incompatibly, one as a reversal and one as a clarification. "I meant" may say something about intent, but intent is not recoverable from the thread; the effect on the recorded ruling was a reversal, and #933's own recording comment leads with that word. Both sites now say reversed. - "a few minutes later" becomes "five minutes later". 21:32:21Z to 21:37:17Z is 4m56s, and this file is otherwise precise about numbers. - The redundancy comparison preceded the ruling and the 20-call-site check did not, so the two are no longer described as one measurement. The call-site timing is left neutral because the thread does not settle it. Prose only: token sequences identical with strings masked, both differing string tokens are docstrings, the AST with docstrings blanked compares equal, and maximum line length stays 121. --- .../test_enum_lookup_reparent_930_unit.py | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/corekit/test_enum_lookup_reparent_930_unit.py b/tests/corekit/test_enum_lookup_reparent_930_unit.py index 06a4388c3..3220225c0 100644 --- a/tests/corekit/test_enum_lookup_reparent_930_unit.py +++ b/tests/corekit/test_enum_lookup_reparent_930_unit.py @@ -29,18 +29,18 @@ that on the owner's lean toward its first option -- widen both signatures to accept ``default`` and delete the suppression. On GitHub pull request #940, which was implementing that widening, the owner then asked why the two overrides had to exist at -all rather than fall back to the base class's, and after the measurements below showed -them redundant, ruled for the second of the two options laid out there: deleting both -overrides outright rather than widening them. +all rather than fall back to the base class's, and once a comparison against the base +showed them redundant, ruled for the second of the two options laid out there: deleting +both overrides outright rather than widening them. -Measured before acting on that final ruling: neither override ever minted an alias -- +Measured before that final ruling: neither override ever minted an alias -- ``__members__`` and ``list(cls)`` agree at 6 and 4 -- so what each docstring called "Backport support for original codes" was the int-or-name dual resolution :meth:`~pcapkit.corekit.enum.EnumLookup.get` already provides for every other -:class:`int`-valued registry in this tree, and none of the 20 call sites either override -had (all in tests, none in :mod:`pcapkit`) passed a key the base would have resolved -differently. There was nothing left to backport, so ``get``/``get_all`` on both now come -from the base alone, the same as the five classes below that were pure re-parents from the +:class:`int`-valued registry in this tree. Checked when acting on it: none of the 20 call +sites either override had (all in tests, none in :mod:`pcapkit`) passed a key the base +would have resolved differently. There was nothing left to backport, so ``get``/``get_all`` +on both now come from the base alone, the same as the five classes below that were pure re-parents from the start. :class:`ReparentedBasesTests` used to pin, alongside each class's own base-tuple change, that the ``@staticmethod`` decorator survived re-parenting and then the signature widening; now that the method is deleted rather than converted, there is nothing left to @@ -466,12 +466,12 @@ class was ``KeptOverrideQuietnessTests`` while both classes still :data:`sys.tracebacklimit` to ``0`` process-wide on a name miss, unlike the base's own quiet raise. GitHub issue #930 converged both onto that quiet shape instead, settled on GitHub issue #933's follow-up ruling, - in which the owner clarified that the two should follow house - convention and not be loud. (The owner's first answer on the same - issue, a flat refusal, was read as keeping them loud, and an earlier - revision of this file briefly pinned loud as the settled answer on - the strength of that reading; the clarification a few minutes later - superseded it, and what follows is the corrected version.) + in which the owner reversed an earlier answer: the two should follow + house convention and not be loud. (The owner's first answer on the + same issue was a bare refusal, which selected the issue's own numbered + option for keeping them loud, and an earlier revision of this file + pinned loud as the settled answer on that basis; the follow-up five + minutes later reversed it, and what follows is the corrected version.) The first two methods are pinned quiet, and for two different reasons against the tree reverted to before #930. From f01e8625a5604b9920693d744c6bee32f3d4b4ae Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 2 Oct 2026 13:55:21 -0400 Subject: [PATCH 4/4] docs(tests): the call-site check preceded acting, so say so Round 3's review found the previous round had replaced a true sentence with a false one. Splitting the two measurements was right -- the alias comparison is provably pre-ruling, the 20-call-site grep only provably pre-acting -- but "Checked when acting on it" asserts an ordering #940 contradicts: - 01:57:57Z the grep is promised, unconditionally, to be reported before anything is touched; - 02:00:58Z the ruling lands; - 02:01:29Z one comment carries both the grep's result and the announcement that the rewrite is being dispatched. Result and dispatch in the same comment, so the check preceded the acting on every reading: if the grep ran in the three-minute wait it was before the ruling too, and if it ran in the following 31 seconds it still preceded the dispatch. The base text had said "Measured before acting on that final ruling" across both halves, which was accurate for both. - "when acting on it" becomes "before acting on it", keeping the split. - Re-flow the paragraph at 91 columns. One line had been left at 107 where its neighbours run 73-91, the residue of a 179-character line caught mid-edit that came back under the limit without the paragraph being re-wrapped. Lines over 95 characters go from two to one, and the survivor is pre-existing. Prose only: token sequences identical with strings masked, both differing string tokens are docstrings, the AST with docstrings blanked compares equal, and maximum line length stays 121. --- .../test_enum_lookup_reparent_930_unit.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/corekit/test_enum_lookup_reparent_930_unit.py b/tests/corekit/test_enum_lookup_reparent_930_unit.py index 3220225c0..b111ae319 100644 --- a/tests/corekit/test_enum_lookup_reparent_930_unit.py +++ b/tests/corekit/test_enum_lookup_reparent_930_unit.py @@ -33,19 +33,19 @@ showed them redundant, ruled for the second of the two options laid out there: deleting both overrides outright rather than widening them. -Measured before that final ruling: neither override ever minted an alias -- -``__members__`` and ``list(cls)`` agree at 6 and 4 -- so what each docstring called -"Backport support for original codes" was the int-or-name dual resolution +Measured before that final ruling: neither override ever minted an alias -- ``__members__`` +and ``list(cls)`` agree at 6 and 4 -- so what each docstring called "Backport support for +original codes" was the int-or-name dual resolution :meth:`~pcapkit.corekit.enum.EnumLookup.get` already provides for every other -:class:`int`-valued registry in this tree. Checked when acting on it: none of the 20 call +:class:`int`-valued registry in this tree. Checked before acting on it: none of the 20 call sites either override had (all in tests, none in :mod:`pcapkit`) passed a key the base would have resolved differently. There was nothing left to backport, so ``get``/``get_all`` -on both now come from the base alone, the same as the five classes below that were pure re-parents from the -start. :class:`ReparentedBasesTests` used to pin, alongside each class's own base-tuple -change, that the ``@staticmethod`` decorator survived re-parenting and then the signature -widening; now that the method is deleted rather than converted, there is nothing left to -decorate, and :class:`AllSevenInheritTheBareClassmethodTests` covers these two the same way -it always covered the other five. +on both now come from the base alone, the same as the five classes below that were pure +re-parents from the start. :class:`ReparentedBasesTests` used to pin, alongside each +class's own base-tuple change, that the ``@staticmethod`` decorator survived re-parenting +and then the signature widening; now that the method is deleted rather than converted, +there is nothing left to decorate, and :class:`AllSevenInheritTheBareClassmethodTests` +covers these two the same way it always covered the other five. Deleting the overrides is a real behaviour change, deliberately so: each branched on ``isinstance(key, int)`` and routed every other type -- ``None``, a :class:`float`, ... --