Skip to content

Reorder protocol class members to the member-order convention #2490

New issue
New issue
Closed
#2765
Closed
Reorder protocol class members to the member-order convention#2490
#2765
Labels
refactorRestructuring for its own sake — neither a fix nor a new capability (refactor: prefix)
Milestone
PyPCAPKit v1.6

Description

@JarryShaw
JarryShaw
opened on Oct 31, 2026

Reorder protocol class members on main to the member-order convention. Ruled by the owner: HTTP v1 is a violation; dunders (__init__, __new__, __post_init__, __index__, __length_hint__, and the rest) are Data Models; the old protocols (IPv4, TCP, UDP, ARP, Ethernet, IPv6) are the reference.

Order inside a protocol class body: properties, public methods (id, read, make, register_*), dunders (Data Models), then utilities (_read_xxx before its _make_xxx).

Landed violations (ref: audit on 545ad0ca7): application/application.py, application/httpv1.py (HTTP), application/httpv2.py, application/ospf.py, application/ssl.py, data/application/httpv2.py, internet/esp.py, internet/hip.py, internet/ipv6_route.py, internet/ipx.py, internet/mh.py, misc/pcap/frame.py, misc/pcap/header.py, misc/pcapng.py, protocol.py (ProtocolBase), schema/misc/pcapng.py, schema/schema.py, transport/sctp.py.

Open PR-only violations are fixed by each PR's own rework (#2444, #2446, #2465, #2466).

Mechanical reorder only; no behaviour change. Split by file group, one PR per group.

Activity

  1. JarryShaw
    added
    refactorRestructuring for its own sake — neither a fix nor a new capability (refactor: prefix)
    pendingAccepted and unstarted - nobody is on it and nothing blocks it
    on Oct 31, 2026

  2. JarryShaw commented on Oct 31, 2026

    @JarryShaw
    JarryShaw
    on Oct 31, 2026
    OwnerAuthor

    Held: Actions queue at 78 (well above the hold line). The member-order reorder dispatches when the queue drops below 40.

  3. JarryShaw
    added this to PyPCAPKiton Oct 31, 2026
  4. JarryShaw
    moved this to Pending in PyPCAPKiton Oct 31, 2026
  5. JarryShaw
    added
    wipWork in flight - a covering PR is open or an agent is actively on it
    and removed
    pendingAccepted and unstarted - nobody is on it and nothing blocks it
    on Oct 31, 2026
  6. JarryShaw
    moved this from Pending to WIP in PyPCAPKiton Oct 31, 2026
  7. JarryShaw
    added 2 commits that reference this issue on Oct 31, 2026

    refactor(protocols): move HTTP private helpers into Utilities

    fe6ea7f

    refactor(protocols): order application protocol members (#2491)

    487c91c

  8. JarryShaw commented on Nov 1, 2026

    @JarryShaw
    JarryShaw
    on Nov 1, 2026
    OwnerAuthor

    Dispatched now; the Actions queue is below the hold line. The worker re-audits current main, makes one branch per file group, and AST-checks each as a pure reorder. Files that other streams are editing right now are deferred until those land: the LinuxSLL/ARPHRD link files, IGMP, the checksum protocols, DTLS, and IPX/Loopback/SCTP for the registry work.

  9. JarryShaw commented on Nov 1, 2026

    @JarryShaw
    JarryShaw
    on Nov 1, 2026
    OwnerAuthor

    Decisions needed for the rest of this issue. protocol-member-order.rst does not settle these:

    1. Helpers that neither read() nor make() calls and no registry reaches (e.g. HTTPv1._decode_text, PCAPNG._check_block_floor, IPv6._exthdr_key): where do they go? Options: (a) last in Utilities, (b) next to their first caller. Recommendation: (a).
    2. Rule c, ordering specific readers and makers by enum value, is not applied anywhere yet. Options: (a) apply it, with the registry default first, as IPv4 has it; (b) drop rule c for registry-table order. Recommendation: (a), in its own PRs.
    3. Registry fallbacks that read()/make() name directly (_read_*_unknown) were treated as specific readers, as refactor(protocols): order application protocol members #2491's review did. Options: confirm that, or file them as helpers. Recommendation: confirm, and write it on the page.
    4. ARP, a reference protocol, is reordered on Reorder protocol class members to the member-order convention #2490's link branch, because its read/read/make/make layout breaks rule b. Options: keep the change, or drop it and record ARP as an exception. Recommendation: keep.
    5. A guard test that runs the group-order audit with a known-exceptions table. Recommendation: add it now; the reviewer showed it can go green with the deferred files listed.
  10. JarryShaw
    added
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Nov 1, 2026

  11. JarryShaw commented on Nov 1, 2026

    @JarryShaw
    JarryShaw
    on Nov 1, 2026
    OwnerAuthor
    1. (a)
    2. (a)
    3. confirm
    4. keep
    5. add it now

  12. JarryShaw commented on Nov 1, 2026

    @JarryShaw
    JarryShaw
    on Nov 1, 2026
    OwnerAuthor

    Owner took all five recommendations, paraphrased:

    1. A helper that neither read() nor make() calls and no registry reaches goes last in Utilities.
    2. Rule c applies: specific readers and makers in enum-value order, with the registry default first, as IPv4 has it. It lands in its own PRs.
    3. Registry fallbacks that read()/make() name directly (_read_*_unknown) count as specific readers, and the page says so.
    4. ARP's reorder on the link branch stays.
    5. A guard test is added now.

    Reconciled: items 1, 2 and 3 are written onto protocol-member-order.rst, and item 5's guard test is dispatched now in one PR. That guard carries a known-exceptions table, so it passes on main today and keeps passing as #2522, #2523 and the held link/misc branches merge. Applying item 1's moves and rule c is another mechanical reorder of the same files, so it follows in its own PRs once the in-flight streams release them (the checksum, TLS, HTTP, IGMP and link streams).

  13. 30 remaining items

  14. JarryShaw commented on Nov 4, 2026

    @JarryShaw
    JarryShaw
    on Nov 4, 2026
    OwnerAuthor

    Re-sequenced: this now runs last, on a quiet tree, after #2717, #2749 and #2730. Those touch protocol modules that a member reorder would move, and the reorder can be re-run cleanly after them.

  15. JarryShaw commented on Nov 4, 2026

    @JarryShaw
    JarryShaw
    on Nov 4, 2026
    OwnerAuthor

    Unblocked. #2717 (#2759), #2749 (#2755) and #2730 (#2757) have all merged, and no open PR edits pcapkit/protocols/ now; #2483 only touches the changelog. A worker is applying the three remaining parts of protocol-member-order.rst on main c211805ae:

    • rule c: option readers in their registry's declaration order;
    • subgroup d: helpers that read/make do not reach go last;
    • registry-map entry order.

    Each module gets an AST-identity proof that the reorder is pure. The guard is extended so that it checks these rules too.

  16. JarryShaw
    added
    wipWork in flight - a covering PR is open or an agent is actively on it
    and removed
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Nov 4, 2026
  17. JarryShaw
    moved this from Blocked to WIP in PyPCAPKiton Nov 4, 2026

  18. JarryShaw commented on Nov 4, 2026

    @JarryShaw
    JarryShaw
    on Nov 4, 2026
    OwnerAuthor

    Decision needed: #2765 surfaced five wording questions on protocol-member-order.rst. The PR implements the recommendations, so a ruling only adjusts the page.

    1. A class with several registries (TCP nests MPTCP, SCTP has a block per registry, MH/PCAPNG list all readers before all makers). Rec: use the MH/PCAPNG shape, which is the page's literal "all _read_*() before all _make_*()". The guard checks only within each registry.
    2. PCAPNG.__option__ order: namespace then value, or sorted by opt_value? Rec: namespace then value, and write that on the page.
    3. Int keys with no enum: the page says "alphabetically". Rec: reword it to "by key value", which is what TCP already does.
    4. The PCAP-NG Option.__enum__: Rec: out of scope, because Option.register's warning relies on its order.
    5. The .rst member lists (e.g. IPv4's) no longer follow source order. Rec: a separate docs-only follow-up.
  19. JarryShaw
    added
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Nov 4, 2026
  20. JarryShaw
    moved this from WIP to Needs decision in PyPCAPKiton Nov 4, 2026
  21. JarryShaw
    added a commit that references this issue on Nov 4, 2026

    refactor(protocols): order option readers, unreachable helpers and re…

    a3ba9bf
  22. JarryShaw
    closed this as completedin #2765on Nov 4, 2026
  23. github-project-automation
    moved this from Needs decision to Done in PyPCAPKiton Nov 4, 2026
  24. JarryShaw
    removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Nov 4, 2026

  25. JarryShaw commented on Nov 4, 2026

    @JarryShaw
    JarryShaw
    on Nov 4, 2026
    OwnerAuthor

    Closed by #2765. The five wording questions above were not ruled on before it merged, so they moved to #2772, which keeps needs: decision.

  26. JarryShaw
    added a commit that references this issue on Nov 4, 2026

    docs(changelog): fold #2764, #2765, #2766 and #2767 (#2449)

    a066345
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

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

    Projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions