Repository navigation
refactor(protocols): a protocol class takes its module's name (igmpv1.IGMPv1, httpv1.HTTPv1) #2529
Description
Owner ruling (2026-11-01, in session), paraphrased: a protocol class is named after its module, whatever its canonical name and id(). So igmp.IGMP stays the base, and the members become igmpv1.IGMPv1, igmpv2.IGMPv2 and igmpv3.IGMPv3. IGMPv1's id() stays ('IGMP',).
Protocol modules on main whose class does not match (AST audit at c43cd1a33, schema/ and data/ excluded):
internet/igmpv1.py,igmpv2.py,igmpv3.py:IGMP→IGMPv1,IGMPv2,IGMPv3. These are new in 1.6.0, so the change is not breaking. Dispatched now.application/httpv1.py,httpv2.py:HTTP→HTTPv1,HTTPv2. These shipped in 1.5.1, so the change is breaking forfrom pcapkit.protocols.application.httpv1 import HTTP. It is folded into refactor(application): use the RFC names HTTP/1 and HTTP/2 in HTTP's id()? #2524's PR, which already changes HTTP'sid()and carries the breaking label.misc/null.py:NoPayload.
Decisions needed:
- Does the rule cover
misc/null.NoPayload? Options: (a) exempt it, since it is the no-payload sentinel rather than a protocol, and record the exception; (b) rename the module tonopayload.py; (c) rename the class toNull. Both (b) and (c) are breaking. Recommendation: (a). - Do
schema/anddata/modules follow the same rule for classes named after a module? Recommendation: yes, in the same PRs.
The rule is written onto protocol-family-grouping.rst's No Invented Suffixes section as part of the IGMP change.
Activity
JarryShaw commentedon Nov 1, 2026
Q3 (owner, in session): move a module's second protocol class into its own module? This is the counterpart of the naming rule: one protocol class per module, named after it. An introspection audit of main lists every module that exports more than one protocol class:
link/arp.py:ARP,InARP(both shipped in 1.5.1) →link/inarp.py.application/rarp.py:RARP,DRARP(both shipped in 1.5.1) →application/drarp.py.application/tls.py:TLS,SSLv3(new in 1.6.0) →application/sslv3.py.protocols/protocol.py:Protocol,ProtocolBase. These are the framework bases, not dissectors.Options: (a) move all three and exempt
protocol.py; (b) move onlySSLv3(not breaking); (c) leave them. Recommendation: (a). Under the #1715/#1103 ruling,InARPandDRARPmove with no re-export at the old path. That makes itbreaking, with!in the subject andversionremoved:: 1.6.0for the old paths; the top-levelpcapkit.protocolsnames are unchanged.SSLv3moves with no directive.Sequencing if (a):
DRARPcan go at once.InARPwaits for #2490's link reorder ofarp.py, andSSLv3for the #2517/#2525 PR, which is editingtls.py.JarryShaw commentedon Nov 1, 2026
Owner ruling on Q3 (in session), paraphrased: take (a). A module holds one protocol class, named after it, and the conventions record that. So
InARPmoves tolink/inarp.py,DRARPtoapplication/drarp.pyandSSLv3toapplication/sslv3.py, andprotocol.py's framework bases are exempt.InARPandDRARPmove with no re-export (breaking,!,versionremoved:: 1.6.0for the old paths);SSLv3moves with no directive.Reconciled: both rules go onto protocol-family-grouping.rst's No Invented Suffixes section in the IGMP rename PR.
DRARPis dispatched now.InARPwaits for #2490's link reorder ofarp.py, andSSLv3for the #2517/#2525 PR. Q1 (misc/null.NoPayload) and Q2 (schema/anddata/modules) are still open, with the recommendations above unchanged.JarryShaw commentedon Nov 1, 2026
Q2 - yes,
schemaanddatashould keep consistency with the main protocol layout.on a side note, i'm considering move from
protocols.schema/data.<layer>.<protocol>andprotocols.<layer>.<protocol>pairing toprotocols.<layer>.<protocol>.schema/data/protocolgroups. what's your opinion from a repo structure perspective? (if not considering path and API breaking changes)JarryShaw commentedon Nov 1, 2026
Q2, owner ruling: yes.
schema/anddata/modules keep the same layout as the protocol modules, covering both the naming rule and one class per module. It applies to the IGMP rename (already in its scope), the DRARP, InARP and SSLv3 moves, and HTTP's renames on #2524. Q1 (NoPayload) is still open.On grouping by protocol (
protocols/<layer>/<proto>/{protocol,schema,data}.pyinstead of three parallel trees), setting breaking changes aside, I'd do it:<layer>/<proto>path.Costs to plan for:
schema/schema.py,data/data.py,protocol.py, shared field types) stay in a core location.protocols.schemaandprotocols.datawould become pure aggregations, which refactor!: remove every surviving re-export of a moved name or module #1744 allows, or go away.ModuleDescriptorpath strings, the docs tree and the aggregators. That is a codemod: best done as one change after this wave lands, piloted on one family to measure import cycles and the docs build.Naming under it: the package takes the protocol's name, its modules take their role, and each module holds the class named after the package. That is #2529's rule, restated per package.
JarryShaw commentedon Nov 1, 2026
yea, then let's do the move.
on the costs side,
protocols.{protocol,data,schema}top levelJarryShaw commentedon Nov 1, 2026
Q1 - rename it, i think the new name should be
no_payload.pyJarryShaw commentedon Nov 1, 2026
Owner rulings, paraphrased and reconciled:
protocols.{protocol,schema,data}; there are no re-exports; and it is one codemod after this wave, piloted first. Tracked as refactor(protocols)!: group each protocol's protocol, schema and data into one package #2534, where the settled design is written down. The VLAN-family pilot is dispatched now.misc/null.pytono_payload.py. It shipped in 1.5.1 and 64 files import it, so renaming it now would collide with nearly every open PR. It is scheduled inside refactor(protocols)!: group each protocol's protocol, schema and data into one package #2534's codemod, which rewrites those imports anyway. Say so if you want it sooner, as its ownbreakingPR.Every question on this issue is now answered. Still to do here: the IGMP rename (ready locally, held for the queue), the HTTP renames with #2524, the DRARP move (in progress), and the InARP and SSLv3 moves after their files free up.
JarryShaw commentedon Nov 1, 2026
Owner ruling (in session), paraphrased: IGMP's
info_namefollows the class. A parsed message sits underigmpv1,igmpv2origmpv3in its enclosing layer's data, as ICMPv4/v6, IPv4/v6 and LinuxSLL1/2 do. The IGMP rename commit (92fd4ca9f, held for the queue) already does that, and a test pins it, so nothing changes.JarryShaw commentedon Nov 1, 2026
Q4, from the IGMP rename's review: how does Q2 read for
schema/anddata/modules? A schema module legitimately holds several classes, one per sub-structure:schema/internet/igmpv3.pyhas five. So "a module holds one protocol class" cannot apply to it literally.Recommendation: 1. It keeps each schema next to its parts, and matches what #2534's per-protocol packages will hold. The page will also note that the remaining exceptions (HTTP, InARP, SSLv3, NoPayload) are pending under this issue, so its present-tense rule is not read as already true everywhere.
11 remaining items
refactor(link)!: move InARP into its own module (#2596)
refactor(application): move SSLv3 into its own module (#2529)
JarryShaw commentedon Nov 2, 2026
#2602 has merged, so SSLv3 lives in
application/sslv3.py. HTTP (#2544) and InARP (#2596) had already moved. The one exception left on this issue isNoPayload. Per Q1 and Q4,misc/null.pybecomesmisc/no_payload.py, and itsschema/anddata/modules move with it, with no re-export.Blocked: that rename rewrites import paths and cross-references across much of the tree, and #2561's whole-repo citation sweep is still being written over the same files. The rename is scripted, so running it after #2561 merges costs nothing, while running it now would force both to rebase. It is dispatched as soon as #2561 merges.
JarryShaw commentedon Nov 2, 2026
#2561 has merged as #2626. Unblocked. A worker is moving
NoPayloadand itsschemaanddatamodules frommisc/null.pytomisc/no_payload.py, with no re-export. The move is breaking, since all three shipped in v1.5.1. The rename is done by a script, so a conflicting rebase is resolved by re-running the script. Once it lands, protocol-family-grouping.rst's pending list is empty.refactor(protocols)!: move NoPayload into misc/no_payload.py (#2529)
refactor(protocols)!: move NoPayload into misc/no_payload.py (#2529) (#…