Skip to content

refactor: move helpers shared outside the schema layer into corekit or utilities #2674

Closed

Description

@JarryShaw

The owner ruled in session that pad_length, and every other helper used outside the schema layer, moves to pcapkit.corekit or pcapkit.utilities.

Known so far:

  • pad_length is in pcapkit.corekit.fields.strings. Callers outside the fields module: the TCP, DCCP and SCTP makers, PCAP-NG, the PCAP-NG and Scapy engines, and the HIP and SCTP schemas. It is pure arithmetic and imports nothing from pcapkit, so under utilities-or-corekit.rst it passes the utilities test.
  • keep_short_read and replay_short_read are in pcapkit.protocols.schema.schema. Callers: the Application, Internet, Link and Transport bases, the PCAP Header and IGMPv3. Both act on protocols and their schemas, so they belong in corekit. Both shipped in v1.5.1, so their old path gets a versionremoved note and no re-export.

To do:

  • Audit pcapkit/corekit/fields/ and pcapkit/protocols/schema/ for every other module-level helper called from outside them.
  • Move each one to the home utilities-or-corekit.rst gives it, and update every caller.
  • Record the rule on utilities-or-corekit.rst.
  • Add an import-boundary test that fails when a helper defined in a schema or fields module is called from outside the schema layer.

Blocked until two things merge: the #2614 internet group's PR, which edits pad_length's callers in the IPv6 extension, MH, HIP and IPv4 modules, and #2665's PR, which edits replay_short_read.

Activity

  1. added this to the PyPCAPKit v1.6 milestone on Nov 2, 2026
  2. added
    refactorRestructuring for its own sake — neither a fix nor a new capability (refactor: prefix)
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Nov 2, 2026
  3. moved this to Blocked in PyPCAPKiton Nov 2, 2026
  4. JarryShaw commented on Nov 2, 2026

    @JarryShaw
    OwnerAuthor

    #2677 and #2679 have merged, so this is unblocked. A worker is on it now. Moved from blocked to wip.

  5. 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 2, 2026
  6. moved this from Blocked to WIP in PyPCAPKiton Nov 2, 2026
  7. JarryShaw commented on Nov 2, 2026

    @JarryShaw
    OwnerAuthor

    Needs deciding: do helpers that encode one protocol's own wire format move too? The worker moved every helper the field and schema.schema modules share outside the schema layer: pad_length, parse_ip_address, packet_byteorder, keep_short_read and replay_short_read. It left in place those defined in a protocol's own schema module: jumbo_payload_length and jumbogram_error (IPv6), the IPv6-Route length helpers, HIP's parameter_total_len, sslv2_record_length, DCCP's header_length, and three PCAP-NG helpers. Their own protocol class calls them, and ip_pseudo and toolkit.scapy also call jumbo_payload_length.

    1. Keep them where they are, and record the carve-out on utilities-or-corekit.rst.
    2. Same as 1, but move jumbo_payload_length into corekit, since protocols other than IPv6 call it.
    3. Move them all.

    Recommendation: 1. They encode one protocol's format, so neither corekit nor utilities fits them, and your earlier ruling on the shared value helpers kept protocol-specific encoders where they are. I'm going ahead with 1 unless you say otherwise.

  8. added
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Nov 2, 2026
  9. moved this from WIP to Needs decision in PyPCAPKiton Nov 2, 2026
  10. JarryShaw commented on Nov 2, 2026

    @JarryShaw
    OwnerAuthor

    i say maybe 2. if a helper is used elsewhere, there's a strong reason to extract it out so that other implementors can reference and reuse. but only if the helper is really something generic and can be reused.

  11. JarryShaw commented on Nov 2, 2026

    @JarryShaw
    OwnerAuthor

    The owner chose 2, paraphrased: a helper used outside its own protocol moves out, so other implementors can find and reuse it, but only when it is genuinely generic.

    • jumbo_payload_length moves, since ip_pseudo, toolkit.scapy and an example generator use it. Its home is chosen by utilities-or-corekit.rst.
    • Helpers only their own protocol calls stay put: jumbogram_error, the IPv6-Route, HIP, SSLv2, DCCP and PCAP-NG length helpers.
    • The carve-out sentence on utilities-or-corekit.rst is rewritten to state the rule: a helper used outside its own protocol and generic enough to reuse moves; one used only by its own protocol stays.
    • The worker also re-checks every protocol-schema helper for callers outside its protocol.

    This settles the question, so needs: decision comes off. It stays wip.

  12. removed
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Nov 2, 2026
  13. moved this from Needs decision to WIP in PyPCAPKiton Nov 2, 2026
  14. JarryShaw commented on Nov 2, 2026

    @JarryShaw
    OwnerAuthor

    The PR is #2704. A Sonnet review is running.

  15. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Nov 3, 2026
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

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions