Skip to content

docs: #719-style prose sweep after the #1090–#1102 defect wave #1103

Open

Description

@JarryShaw

Is your feature request related to a problem? Please describe.
The defect wave #1090–#1102 changes code and docstrings in several modules, and #719's two rounds of prose sweeps predate those changes. The maintainer asked for another #719-style pass over the docs and comments once the wave lands.

Describe the solution you'd like
Re-read the prose that the wave's merged PRs touched or made stale: docstrings, comments, docs/ and pep.rst. Apply #719's rules: no churn, accuracy first, no timed context, and only prose changes, with an AST guard on every .py.

Blocked until
Every issue in #1090–#1102 is closed.

Describe alternatives you've considered
Folding the sweep into each fix PR was rejected: thirteen parallel PRs editing shared prose would conflict.

Activity

  1. added
    docsPull requests that change documentation only (docs: subject prefix)
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Oct 6, 2026
  2. JarryShaw commented on Oct 6, 2026

    @JarryShaw
    OwnerAuthor

    Add to this sweep: ProtocolBase.pack() (protocol.py:559-576) rebuilds from make(**kwargs) and ignores the instance's parsed or constructed state. A bare X(...).pack() therefore returns defaults. Neither the docstring nor the rst docs say so. This came up in the #1143 review.

  3. JarryShaw commented on Oct 6, 2026

    @JarryShaw
    OwnerAuthor

    Add to this sweep, after #1147 merges:

    • docs/source/ext.rst:~266 says from_data "warns UnknownFieldWarning" and that its keywords "come from your _make_data". Caller-supplied keywords now raise UnsupportedCall.
    • The _check_construction_keywords docstring describes kwargs as what remains after __init__. from_data calls it as well now.
  4. JarryShaw commented on Oct 6, 2026

    @JarryShaw
    OwnerAuthor

    Still blocked, with a new condition. Every issue in #1090–#1102 is closed, but the sweep is meant to run after all the defects the wave turned up are fixed, so that it reads the final docstrings once. It now unblocks when #1127, #1133, #1144, #1151, #1154, #1155 and #1156 are all closed.

  5. JarryShaw commented on Oct 7, 2026

    @JarryShaw
    OwnerAuthor

    Still blocked, with a new condition. All seven issues named in the last condition are now closed. Two more defects from the same audit are now being fixed, #1165 and #1167, and they edit extraction.py, dumpkit/pcap.py, ipv4.py and udp.py. The sweep unblocks once #1165 and #1167 are both closed, so it reads the final docstrings once.

  6. JarryShaw commented on Oct 7, 2026

    @JarryShaw
    OwnerAuthor

    Condition extended: the #1169 cross-review turned up #1170, a defect in the same class that touches ipv4.py, ipv6.py, udp.py and the shared _import_next_layer. The sweep now unblocks once #1165, #1167 and #1170 are all closed.

  7. JarryShaw commented on Oct 7, 2026

    @JarryShaw
    OwnerAuthor

    The previous condition is met: #1165, #1167 and #1170 are all closed. Two defects from the same audit are still in flight: #1172 (l2tpv2.py) and #1173 (ipv6_route.py, mh.py, application.py). The sweep would read the docstrings of the files they edit, so it now unblocks when #1172 and #1173 are both closed. These are the last known items from this audit.

  8. JarryShaw commented on Oct 7, 2026

    @JarryShaw
    OwnerAuthor

    Unblocked: #1172 and #1173 are both closed (#1174 and #1176 merged at 04:00Z and 04:01Z). Now pending. Three later items are still open: #1175 (l2tpv2.py), #1177 (ipv6.py) and #1178 (ipv6_ext.py). The sweep skips those three files and they get a delta pass once each issue closes.

  9. added
    pendingAccepted and unstarted - nobody is on it and nothing blocks it
    and removed
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Oct 7, 2026
  10. JarryShaw commented on Oct 7, 2026

    @JarryShaw
    OwnerAuthor

    Dispatched as three prose slices, each scoped to git diff b10e7951b origin/main: protocols, schema-data and foundation-docs. The #1175/#1177/#1178 files are excluded.

  11. 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 7, 2026
  12. 192 remaining items

  13. added 2 commits that reference this issue on Oct 28, 2026
  14. JarryShaw commented on Oct 28, 2026

    @JarryShaw
    OwnerAuthor

    PR #2415 (round 2 of this sweep) converted every pcapkit.protocols.protocol.Protocol.<member> cross-reference under pcapkit/ to ProtocolBase.<member>, since that's the class that actually defines them — Protocol itself is just a docstring + __init_subclass__. It deliberately left docs/source/ and tests/ untouched (31 occurrences) because the conversion has a real rendering effect: in the built Sphinx docs, a Protocol.X link lands on the page's full member documentation, while ProtocolBase.X lands on a docstring-less stub that links onward. The cross-review measured this directly — the conversion moves 63 rendered links from full entries to stubs (Protocol.<m> hrefs go from 154 to 91 in the built HTML), and _decode_next_layer's docs page shrinks from a 3.7 KB docstring to a 14 KB signature list.

    Decision needed: which direction is correct?

    1. Restructure protocol.rst so ProtocolBase hosts the full member entries (matching where the members are actually defined), then finish converting pcapkit/, docs/ and tests/ together — this is technically correct but is a real doc-architecture change, not a drop-in fix.
    2. Reverse this round's pcapkit/ conversion back to Protocol.X, since that's already where the full docs render, and leave the Sphinx rendering structure as-is.

    My recommendation (matching the cross-reviewer's): option 1. ProtocolBase is the real defining class, and restructuring protocol.rst once is cleaner than permanently pointing cross-references at the wrong class to work around a docs-page layout. But this changes what a reader lands on when following these links, which is worth your call before #1103 round 3 touches docs//tests/ on this question.

  15. added
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 28, 2026
  16. moved this from WIP to Needs decision in PyPCAPKiton Oct 28, 2026
  17. JarryShaw commented on Oct 28, 2026

    @JarryShaw
    OwnerAuthor

    ProtocolBase is the internal hidden API. Protocol is the one exposing the APIs and attributes. So i'd prefer we keep referring on Protocol whenever possible.

  18. JarryShaw commented on Oct 28, 2026

    @JarryShaw
    OwnerAuthor

    Got it — prefer Protocol in prose/cross-references wherever a member is reachable through it, even though it's actually defined on ProtocolBase. This reverses #2415's main conversion (which went the other way) and the smaller version of the same conversion #2403... correction, #2408 already merged in the register docs. Closing #2415 rather than reworking it (its core premise is now invalid) and will open a corrective PR reverting the merged change, plus fold the standardization onto Protocol into the next #1103 round.

  19. removed
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 28, 2026
  20. moved this from Needs decision to WIP in PyPCAPKiton Oct 28, 2026
  21. JarryShaw commented on Oct 28, 2026

    @JarryShaw
    OwnerAuthor

    New refinement on the Protocol/ProtocolBase cross-reference rule: for the sites #2420 reverted to bare ProtocolBase.X, use the split Sphinx form :meth:Protocol.X <...ProtocolBase.X>`` instead — display text always says Protocol.X (for consistency), but the target points at `ProtocolBase.X` since that's where the actual documentation entry lives. Applying this to #2420's 21 reverted sites, and auditing whether this should extend to other `ProtocolBase.X` targets across the tree for the same reason.

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

    docsPull requests that change documentation only (docs: subject prefix)wipWork in flight - a covering PR is open or an agent is actively on it

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions