Skip to content

refactor(protocols): order option readers, unreachable helpers and registry maps per the member-order convention (#2490) - #2765

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2490-member-order-rest
Nov 4, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/2490-member-order-rest

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • refactor — restructures code without changing behaviour

Description of your pull request and other information

This applies the three parts of protocol-member-order.rst that no module followed yet. It is a mechanical reorder of 23 modules, with no behaviour change.

  • Rule c: each registry's option readers, then its makers, open with the default and follow enum-value order (IPv4, HOPOPT/IPv6-Opts, HIP, MH, PCAPNG, …).
  • Subgroup d: helpers that read(), make() and the registries never reach go last. This touches 18 classes.
  • Registry maps: 13 maps are now in enum-value order, int keys ascending. PCAPNG.__option__ and the PCAP-NG Option.__enum__ are left alone: one is computed, and Option.register's warning depends on the other's order.
  • Purity: each module has the same statements, class-body AST multiset (dicts compared as mappings) and non-blank-line multiset as main. All 1198 class-level dunder dicts are equal at runtime.
  • Guard: the member-order guard gains three rules (specific, unreached, registry) and keeps an empty KNOWN_EXCEPTIONS. Run against main's pcapkit/, it fails 6 times. Each rule was also proven against mutated copies.
  • Checks: every plain-unittest leg passes, and the docs build shows 0 new warnings.

Closes #2490

…gistry maps per the member-order convention (#2490)

Move members and map entries only; every member's text is unchanged.

- Rule c: each registry's readers, then its makers, open with the default and
  follow the registry's enum-value order (IPv4, HOPOPT, IPv6_Opts, HIP, MH, PCAPNG).
- Subgroup d: utilities read(), make() and the registries do not reach move
  last, keeping their order (HTTPv1, IPv6, TCP, SCTP, MH, PCAPNG and others).
- Registry maps in enum-value order, int keys ascending: Internet, Link, FRAD
  and UDP __proto__, IPv4/HOPOPT/IPv6_Opts/MH __option__, HIP __parameter__,
  ICMPv4 __type__, MH __message__, PCAPNG __block__ and __secrets__.
- The member-order guard checks rule c, subgroup d and registry-map order,
  with KNOWN_EXCEPTIONS still empty; the page's guard paragraph says so.
@JarryShaw JarryShaw added refactor Restructuring for its own sake — neither a fix nor a new capability (refactor: prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Nov 4, 2026
@github-actions github-actions Bot added this to the PyPCAPKit v1.6 milestone Nov 4, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Nov 4, 2026
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Coverage: 94.45% (unit tier, Python 3.15, ce5b572c5, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 106 4 20 2 95.24%
pcapkit/const 20918 175 2908 156 98.57%
pcapkit/corekit 2895 76 930 37 96.84%
pcapkit/dumpkit 286 3 102 3 98.45%
pcapkit/foundation 3507 27 1216 32 98.75%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 22764 155 5918 127 99.00%
pcapkit/toolkit 961 26 342 8 96.62%
pcapkit/utilities 559 7 150 5 98.31%
pcapkit/vendor 5590 2286 1280 214 55.75%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at ce5b572c5 (Sonnet cross-review, and my own purity check)

  • Purity: for all 23 modules, the class-body ASTs (dicts compared as mappings) and the non-blank lines match main exactly. No code iterates any reordered map in insertion order. Comments stayed next to their members.
  • Conformance: the moves follow protocol-member-order.rst. Spot-checked IPv4, HOPOPT, TCP, MH and PCAPNG, plus the Internet, Link, UDP and FRAD maps by hand.
  • Guard: it passes here and fails 6 times on main. A swapped pair of IPv4 readers fails it too. It parses as Python 3.8.
  • CI: Project tests, Lint and deploy-pages pass. The dispatch, option and schema round-trip suites pass as well.
  • Wording: the five page questions are on Reorder protocol class members to the member-order convention #2490. Question 3 ("alphabetically" vs "by key value") is the only one where the page wording doesn't yet match what this PR does.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Nov 4, 2026
@JarryShaw
JarryShaw merged commit a3ba9bf into main Nov 4, 2026
74 of 75 checks passed
@JarryShaw
JarryShaw deleted the fix/2490-member-order-rest branch November 4, 2026 17:15
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Nov 4, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Nov 4, 2026
JarryShaw added a commit that referenced this pull request Nov 4, 2026
- #2766 (Fixed): a cut list of plain values ends with the data, where 1.5.x
  read zero values past the cut (a cut Type 0 Routing header, measured on
  v1.5.1), into the ListField entry; IGMPv3's sources and the Query's
  num_sources, into the IGMP entry.
- #2767: a cut header rebuilt on its own reads back its parsed values, a
  regression of the 1.6.0 rebuild path, into the cut-header Fixed entry; the
  round-trip test rule, into the conventions entry.
- #2764: SSLv2, TLS and DTLS records rebuild from their to_dict(), into the
  TLS/DTLS Added entry, as those classes never shipped.
- #2765: the further member reorders and guard rules, into the member-order
  entry, which records the earlier #2490 reorders.
- CHANGELOG.md regenerated; util/changelog_md.py --check passes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Restructuring for its own sake — neither a fix nor a new capability (refactor: prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Reorder protocol class members to the member-order convention

1 participant