Skip to content

docs(protocols): second-round pass over the link, misc, transport and root protocol prose (#719) - #1087

Merged
JarryShaw merged 1 commit into
mainfrom
docs/719-r2-link-misc-transport
Oct 6, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
docs/719-r2-link-misc-transport

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

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
  • chore — anything else

Description of your pull request and other information

Second-round #719 pass over pcapkit/protocols/{link,misc,transport}/, protocols/__init__.py and protocols/protocol.py. Only prose the code contradicts, timed context, typos and broken references changed; 14 of 31 files touched.

  • Wrong claims fixed: Ethernet and VLAN offset tables; TCP table listed an unparsed tcp.flags.ns; PCAP-NG pack_flags/pack_hash rows named the epb_* methods; unquote's default errors is 'replace', not 'strict'; __repr__/__str__ examples; MP_JOIN Raises:; isb_usrdeliv argument; Transport.register's protocol; "values are tuples" registry comments; L2TPv2's "every non-raising __index__" (PCAP Frame/PCAPNG return a frame number).
  • Timed context cut: L2TPv2 (version, dispatch comment), C_Tag.id, the PCAP frame-length commit history.
  • L2TP module headings brought to Title Case (no references to them anywhere).

Checks: AST guard below; Sphinx -n on fresh output dirs, 136 warnings for these files on branch and on main, none new; tests/project 379 passed; tests/test_docstring_contract.py and 27 slice modules (one per process) pass. Lint not run.

File AST identical
pcapkit/protocols/link/__init__.py True
pcapkit/protocols/link/arp.py True
pcapkit/protocols/link/c_tag.py True
pcapkit/protocols/link/ethernet.py True
pcapkit/protocols/link/l2tp.py True
pcapkit/protocols/link/l2tpv2.py True
pcapkit/protocols/link/vlan.py True
pcapkit/protocols/misc/pcap/frame.py True
pcapkit/protocols/misc/pcap/header.py True
pcapkit/protocols/misc/pcapng.py True
pcapkit/protocols/protocol.py True
pcapkit/protocols/transport/__init__.py True
pcapkit/protocols/transport/tcp.py True
pcapkit/protocols/transport/transport.py True

… root protocol prose (#719)

Correct claims the code contradicts: the Ethernet and VLAN field-offset
tables, the TCP table's parsed `tcp.flags.ns` row, the PCAP-NG
`pack_flags`/`pack_hash` method references, `unquote`'s default `errors`,
the `__repr__`/`__str__` examples, `_read_mptcp_join`'s Raises condition,
the `isb_usrdeliv` argument, `Transport.register`'s `protocol` argument,
the "values are tuples" registry comments, and the "every non-raising
`__index__`" claim in L2TPv2.

Cut timed context in L2TPv2, C_Tag and the PCAP frame-length note; fix
typos and re-case the L2TP module headings.

Docstrings and comments only; the AST guard is clean for every file.
@JarryShaw JarryShaw added docs Pull requests that change documentation only (docs: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one review: running A cross-review is in flight against the current head - no verdict yet and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 6, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on fe8731c30: GOOD TO GO (ran on Sonnet; author Opus)

  • AST guard: all 14 files are identical to origin/main once bare strings are stripped.
  • Offsets: the Ethernet src/type offsets (octets 6/12) and the VLAN TCI/type offsets (octets 0/2) match the field widths.
  • TCP:
    • Probed with NS set: flags has no ns key, so dropping that row is right.
    • The _read_mptcp_join Raises: condition matches the code.
    • The ADD_ADDR "IPv4" fix is correct.
  • protocol.py:
    • unquote defaults to replace.
    • The new repr(Frame) shape matches a probe on in.pcap.
    • _read_unpack now cites struct.unpack.
  • __index__: Frame.__index__() returns its number. PCAP-NG returns _fnum only for packet blocks and raises otherwise, which the "every non-raising" qualifier covers.
  • Registries and options:
    • __proto__ values are ModuleDescriptors.
    • register accepts a descriptor or a class.
    • The pack_flags/pack_hash methods exist.
    • C_Tag.id() returns (C_Tag, VLAN).
  • Tests: test_docstring_contract, test_tcp_mptcp_join_flag_ordering_unit and test_link_unit pass.

Nit, not raised by this PR: _check_block_floor says a 1–3 octet remainder is caught there, but the floor is < 12. Out of scope here.

UNVERIFIED: no Sphinx run, and no standalone PCAP-NG __index__ probe (that behaviour was read from the code).

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

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Coverage: 88.74% (unit tier, Python 3.14, fe8731c30, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18757 1035 2342 846 90.23%
pcapkit/corekit 1874 91 578 22 94.33%
pcapkit/dumpkit 136 0 40 0 100.00%
pcapkit/foundation 2422 143 842 34 92.62%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 15653 187 3942 162 98.19%
pcapkit/toolkit 487 71 144 3 84.15%
pcapkit/utilities 429 4 122 4 98.55%
pcapkit/vendor 4409 2359 1006 158 42.84%

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

@JarryShaw
JarryShaw merged commit 8a2d7de into main Oct 6, 2026
43 checks passed
@JarryShaw
JarryShaw deleted the docs/719-r2-link-misc-transport branch October 6, 2026 17:39
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Pull requests that change documentation only (docs: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant