Skip to content

refactor(protocols): reorder internet class members to the member-order convention (#2490) - #2523

Open
JarryShaw wants to merge 1 commit into
mainfrom
refactor/2490-member-order-internet
Open

JarryShaw wants to merge 1 commit into
mainfrom
refactor/2490-member-order-internet

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else
  • const — regenerates constant or registry tables, members keeping their numeric values

Description of your pull request and other information

Part of #2490. A pure reorder to protocol-member-order.rst, covering pcapkit/protocols/internet/esp.py, pcapkit/protocols/internet/hip.py, pcapkit/protocols/internet/hopopt.py, pcapkit/protocols/internet/ipsec.py, pcapkit/protocols/internet/ipv6_opts.py, pcapkit/protocols/internet/ipv6_route.py, pcapkit/protocols/internet/mh.py. An AST check confirms the member set and every member body are unchanged; only the order differs. The narrow tests for each touched protocol pass. Files that other streams are editing now are deferred (DTLS, IGMPv3, IPX, OSPF, IPv4, TCP, SCTP, IP_Pseudo). Open questions about cases the page does not cover are posted on #2490.

…er convention (#2490)

- esp: move SecurityAssociation and ESPContext __init__/__repr__ into Data
  models, after Properties and Methods; order ESP's direct helpers by first
  use (_make_opaque, then _payload_bytes).
- hip, hopopt, ipv6_opts: move the options/parameters maker next to its
  reader, ahead of the specific readers.
- ipv6_route, mh: move _make_hdr_ext_len and _mh_message_length out of
  Methods into Utilities, after _make_data; move mh's _pad_mh_message,
  which make() calls, ahead of the message readers.
- ipsec: label the Methods section, which held id() under "Data models".

Pure move: member set, member ASTs and non-banner comments are unchanged.
@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 1, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Nov 1, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review (Sonnet) of all four #2490 branches, this one at 0f42f12f3: GOOD TO GO. The reviewer's own AST checker passes all 17 files: module-level statements are identical, and every class keeps the same member multiset with identical member ASTs and segments. The only non-reorder edits are the two section-banner fixes. Non-blocking: no guard test pins the convention yet, the subjects overstate it (rule c and registry-map ordering are not applied), and the rule-b reading of registry fallbacks is not stated on the page. All three are raised as questions on #2490.

@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 1, 2026
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Coverage: 93.94% (unit tier, Python 3.15, 0f42f12f3, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 20087 144 2710 125 98.78%
pcapkit/corekit 2605 74 818 37 96.52%
pcapkit/dumpkit 281 3 100 3 98.43%
pcapkit/foundation 3194 27 1112 31 98.65%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 19906 213 5236 207 98.32%
pcapkit/toolkit 924 36 334 10 95.55%
pcapkit/utilities 502 7 144 5 98.14%
pcapkit/vendor 5009 2222 1148 182 52.31%

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

This branch has not been deployed

No deployments
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) review: good-to-go Cross-review at the current head says ready; CI state is separate

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

1 participant