diff --git a/docs/source/contributing/conventions/extension-header-subclassing.rst b/docs/source/contributing/conventions/extension-header-subclassing.rst index be81a0816..3da71408c 100644 --- a/docs/source/contributing/conventions/extension-header-subclassing.rst +++ b/docs/source/contributing/conventions/extension-header-subclassing.rst @@ -5,11 +5,12 @@ Extension-Header Base Classes Every IPv6 extension header in this package subclasses :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`. Some name a **second** base -as well, and which ones do is a ruling rather than an accident. The owner ruled on -`#924 `__ that a header usable *only* -as an extension header inherits ``IPv6_Ext`` and nothing else -- ``IPv6_Frag`` being -the example -- while one that is usable as a standalone protocol in its own right -inherits both ``IPv6_Ext`` and ``Internet`` (or ``IPsec``), as ``ESP`` does. +as well, and which ones do is a ruling rather than an accident. The owner ruled, in +review of the rename that made ``IPv6_Ext`` the shared base +(`#917 `__), that a header usable +*only* as an extension header inherits ``IPv6_Ext`` and nothing else -- ``IPv6_Frag`` +being the example -- while one that is usable as a standalone protocol in its own +right inherits both ``IPv6_Ext`` and ``Internet`` (or ``IPsec``), as ``ESP`` does. The family as it stands: @@ -104,10 +105,10 @@ class for it, so nothing implements the classification, but a future one inherit Own-protocolhood on its own is **not** sufficient, and MH is the case that settles it: the alternative reading -- that a protocol in its own right qualifies - whether or not it can appear under IPv4 -- was put to the owner explicitly on - `#924 `__ and not taken, so MH - and ``Shim6`` stay extension-only. A header that is a protocol in its own right - but structurally cannot be an IPv4 payload names ``IPv6_Ext`` alone. + whether or not it can appear under IPv4 -- was put to the owner explicitly in + review of `#917 `__ and not + taken, so MH and ``Shim6`` stay extension-only. A header that is a protocol in its + own right but structurally cannot be an IPv4 payload names ``IPv6_Ext`` alone. Explicit Base Declarations ~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/source/contributing/conventions/mint-criterion.rst b/docs/source/contributing/conventions/mint-criterion.rst index 5121c6c18..3cb73e554 100644 --- a/docs/source/contributing/conventions/mint-criterion.rst +++ b/docs/source/contributing/conventions/mint-criterion.rst @@ -49,7 +49,8 @@ The test, paraphrased from the maintainer's ruling: does the upstream registry t the label as the final, concrete assigned name, or only as a notation for a human reading the table? -Settled on `#847 `__ and reaffirmed +Settled in review of the ``Socket._missing_`` branch-order fix +(`#841 `__) and reaffirmed on `#775 `__ as a core concept of the ruling. @@ -98,9 +99,10 @@ Suffixed Company Names The ethertype case looks like an exception to the rule and is not. The maintainer's reasoning, settled on `#775 `__ after -being raised on `#847 `__: a -proprietary protocol will never have a public name, so the company name is what serves -that purpose in its place. +being raised in review of the same ``Socket._missing_`` fix +(`#841 `__): a proprietary protocol +will never have a public name, so the company name is what serves that purpose in its +place. So the company name is not a note *about* the code -- it is the best name that will ever exist *for* it, which makes it the final concrete assigned name under the test above. diff --git a/docs/source/contributing/conventions/process.rst b/docs/source/contributing/conventions/process.rst index 33751f075..05d22a276 100644 --- a/docs/source/contributing/conventions/process.rst +++ b/docs/source/contributing/conventions/process.rst @@ -72,16 +72,16 @@ detail of what actually changed in that version bump. Ruled on Two different things get confused here, so they are named apart: -* **A pull request's commits.** `#657 `__ - is the shared changelog for the 1.5.0 cycle, and it carries roughly one commit per - code pull request, deliberately unsquashed so that what is and is not accounted for - stays readable in its log. That is a property of *the pull request*, and it is not - what the ruling is about. +* **A pull request's commits.** The shared changelog for the 1.5.0 cycle is a + long-lived pull request of its own, carrying roughly one commit per code pull + request, deliberately unsquashed so that what is and is not accounted for stays + readable in its log. That is a property of *the pull request*, and it is not what + the ruling is about. * **A changelog file's entries.** :file:`docs/source/changelog/1.5.0.rst` groups its entries under a section per top-level module, with ``Added``, ``Changed`` and - ``Fixed`` nested inside each, and a single entry routinely cites several issues at - once -- *"the Mobility Header registry, completed (#383, #437)"* is one bullet, not - two. That is a property of *the file*, and it is the axis the ruling governs. + ``Fixed`` nested inside each, and a single entry routinely cites several changes at + once -- the completed Mobility Header registry is one bullet, not two. That is a + property of *the file*, and it is the axis the ruling governs. Both are measurable rather than matters of memory, which is the point of writing the commands down instead of a figure that will be stale by the next merge: @@ -92,8 +92,11 @@ commands down instead of a figure that will be stale by the next merge: grep -cE '^\* ' docs/source/changelog/1.5.0.rst grep -B1 -E '^-{3,}$' docs/source/changelog/1.5.0.rst | grep -vE '^-{3,}$|^--$' - # commits on the pull request -- a different number, about a different thing - gh pr view 657 -R JarryShaw/PyPCAPKit --json commits -q '.commits|length' + # commits on the shared changelog pull request -- a different number, about a + # different thing + gh pr list -R JarryShaw/PyPCAPKit --state all \ + --search 'shared 1.5.0 changelog in:title' \ + --json commits -q '.[].commits|length' The grouping scheme was settled on #918: **a section per top-level module, with** ``Added``/``Changed``/``Fixed`` **nested inside each** -- module granularity, not @@ -124,8 +127,9 @@ belongs to none. Nor is the map one-to-one with the package list below -- section wants that module's changes; the same prose appearing twice reads as two separate changes. Raised originally on #918. - The restructure itself belongs to #657, which owns the file and merges last; doing - it earlier would conflict with every open change that touches an entry. + The restructure itself belongs to the shared changelog's own pull request, which + owns the file and merges last; doing it earlier would conflict with every open + change that touches an entry. Issue and Pull Request Labels ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -256,39 +260,48 @@ So the question it answers is not "how big is this change" but **"can a caller observe the difference without changing their code"**. On the tree, the changes carrying it are that kind: -* an exception type a caller catches -- ``#811`` raising +* an exception type a caller catches -- + `#805 `__ raising :exc:`~pcapkit.utilities.exceptions.ProtocolError` where a bare - :exc:`struct.error` used to escape, and ``#783`` raising one where a single-bit - lookup used to return a member; -* a public attribute's meaning -- ``#635`` swapping ``Frame.len`` and - ``Frame.cap_len`` between the PCAP and PCAP-NG readers; -* a signature or a name a caller writes -- ``#815`` retyping ``AppType.proto`` and - giving ``register_apptype`` varargs, ``#788`` enforcing ``@final`` at runtime; -* a path a caller or a script depends on -- ``#350`` naming the examples directories - apart. + :exc:`struct.error` used to escape, and + `#759 `__ raising one where a + single-bit lookup used to return a member; +* a public attribute's meaning -- + `#618 `__ swapping ``Frame.len`` + and ``Frame.cap_len`` between the PCAP and PCAP-NG readers; +* a signature or a name a caller writes -- + `#806 `__ retyping + ``AppType.proto`` and giving ``register_apptype`` varargs, + `#778 `__ enforcing ``@final`` + at runtime; +* a path a caller or a script depends on -- the change that named the examples + directories apart. .. warning:: **A pull request's prose and its label can disagree, and the label is not - automatically right.** Both directions have happened here. ``#783`` and ``#811`` - carry the label while their changelog bullets never said so, which a review round - on #657 caught and corrected. ``#848`` carries it too, and its own body argues at - length that the change is *not* breaking -- a review round checked that argument - and found it right on the facts, so there the label is the half that overstates. - So when the two conflict, settle it on what a caller can observe, and fix - whichever of the two is wrong rather than letting the pair stand. + automatically right.** Both directions have happened here. The ``#759`` and + ``#805`` changes carry the label while their changelog bullets never said so, which + a review round on the shared changelog caught and corrected. The + `#844 `__ change carries it too, + and its own pull request argues at length that the change is *not* breaking -- a + review round checked that argument and found it right on the facts, so there the + label is the half that overstates. So when the two conflict, settle it on what a + caller can observe, and fix whichever of the two is wrong rather than letting the + pair stand. .. note:: **The label is not applied uniformly across the repository's history, and a census - that assumes it is will be wrong.** It is dense on pull requests from ``#350`` - upward and effectively absent below: the only earlier carriers are seven - pre-``0.15`` pull requests, ``#3`` to ``#28``, with nothing at all between ``#28`` - and ``#350``. Three of those seven are distribution rollups (``#25``, ``#26``, - ``#28``, each also carrying ``release``); the other four are early - ``refactor``/``feat`` work from before the project stabilised. On issues it is - sparser still, appearing only from ``#775`` up. So ``breaking``'s absence on an old - pull request is weak evidence at best. The current figures, rather than these: + that assumes it is will be wrong.** It is dense on pull requests from the + examples-directory change above onward and effectively absent below it: the only + earlier carriers are seven pre-``0.15`` pull requests, clustered at the very start + of the numbering, with nothing labelled at all between them and that change. Three + of those seven are distribution rollups, each also carrying ``release``; the other + four are early ``refactor``/``feat`` work from before the project stabilised. On + issues it is sparser still, appearing only from ``#775`` up. So ``breaking``'s + absence on an old pull request is weak evidence at best. The current figures, rather + than these: .. code-block:: shell diff --git a/docs/source/contributing/conventions/registry-protocol.rst b/docs/source/contributing/conventions/registry-protocol.rst index 767b787b4..0b07f07ce 100644 --- a/docs/source/contributing/conventions/registry-protocol.rst +++ b/docs/source/contributing/conventions/registry-protocol.rst @@ -211,8 +211,8 @@ guaranteed either: pass a first argument that *is* an instance of the class and delegation **silently succeeds**, so a ``@staticmethod`` override cannot even be relied on to fail loudly. Measured, all three cases, rather than reasoned about. :meth:`~pcapkit.corekit.enum.EnumLookup.get` is itself a ``@classmethod``. The -precedent is `#913 `__, whose -``FEATCode.get`` is ``@classmethod def get(cls, key, default=NO_DEFAULT)`` ending in +precedent is `#903 `__, which made +``FEATCode.get`` a ``@classmethod def get(cls, key, default=NO_DEFAULT)`` ending in ``return super().get(key, default)``; `#908 `__ followed it, which is what turned ``Method.get`` into a classmethod. @@ -263,8 +263,8 @@ lines, with ``--warn-unused-ignores`` reporting neither as unused. **And an override that only reimplements the base is deleted, not repaired.** Widening those two signatures made them faithful copies of the base. Rather than merge them, -the owner asked on `#940 `__ why the -two overrides needed to exist at all, if they could simply fall back to the base's. +the owner asked, in review of that same widening, why the two overrides needed to +exist at all, if they could simply fall back to the base's. They could. Nine cases per class -- name hit, name miss, value hit, value miss and every ``default`` combination -- differed from ``EnumLookup.get.__func__(cls, ...)`` @@ -322,7 +322,7 @@ worked example: since forwards ``default`` verbatim and delegates to ``super().get()``, and that is all it does. It used to convert the base's name-miss :exc:`KeyError` into a :exc:`ValueError` as well, and #923's ruling retired that; the -`#836 `__ refusal to extend the +`#808 `__ refusal to extend the class at all is untouched by the retirement, since only the exception class moved. ``Criticality.get`` went further and no longer exists: conversion was the *only* thing it added over the base, so once that went there was nothing left for an @@ -481,14 +481,14 @@ That leaves the classes with something to decide: audit was taken -- ``FastBindingAcknowledgmentStatus`` and ``IPv6AddressPrefixCode``, for signature reasons (no ``default``, and an :class:`int`/:class:`str` dispatch) - rather than for case -- and - `#940 `__ deleted both as - redundant, per the ruling in the section above; each now inherits ``get`` - from :class:`~pcapkit.corekit.enum.EnumLookup` unchanged. ``LMAAddressCode`` - and ``LocalizedRoutingStatus`` never carried a ``get`` at all, so they had no + rather than for case -- and both were deleted as redundant rather than widened, + on the ruling recorded in the section above and given in review of the widening + itself; each now inherits ``get`` from + :class:`~pcapkit.corekit.enum.EnumLookup` unchanged. ``LMAAddressCode`` and + ``LocalizedRoutingStatus`` never carried a ``get`` at all, so they had no string lookup to fold. ``Criticality`` had one when this audit was taken and no longer does: - `#921 `__ re-parented it onto + `#877 `__ re-parented it onto :class:`~pcapkit.corekit.enum.EnumLookup` and #923 retired the exception conversion that was the override's only remaining job, so it now inherits ``get`` unchanged.