Skip to content

docs(protocols): second-round pass over the schema prose (#719) - #1083

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

JarryShaw merged 1 commit into
mainfrom
docs/719-r2-schema

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • docs — documentation only

Description

Round-two read of all 37 files under pcapkit/protocols/schema/ for #719; 10 changed, 27 left alone. Fixes: claims the code contradicts (EnumMeta naming the wrong class, MH's Pad1 comment saying length 1 while the code sets 0, NoPayload's __init__ rationale, "special-case by name" vs. isinstance), past-tense history in ethernet/hip/pcapng, the IGMP values RFC 7028 §8 gives alongside the MLD ones, two cross-references split across lines, four MH/PCAP-NG attribute comments whose lead phrase rendered as a type, and typos.

Sphinx -n, fresh builds: warnings naming these files drop from 50 to 40 (main → branch), none new. test_docstring_contract.py, tests/project, test_final_enforcement.py and tests/protocols/schema/* pass.

AST guard (docstrings stripped, compared with origin/main):

File Identical
__init__.py True
application/httpv2.py True
internet/hip.py True
internet/ipv4.py True
internet/mh.py True
link/ethernet.py True
misc/null.py True
misc/pcapng.py True
schema.py True
transport/tcp.py True

- schema: EnumMeta documents EnumSchema, not SchemaMeta; fix the
  "enumetaion" typos; repair two cross-references split across lines
- null, ethernet, hip, pcapng: describe the code as it is now rather
  than past revisions or behaviour
- mh: Pad1 comment says length 0, as the code sets; list RFC 7028's IGMP
  protocol values beside the MLD ones; stop four attribute comments
  rendering their lead phrase as a type
- ipv4, httpv2, pcapng, tcp: typos and one-word accuracy fixes
- __init__: mark OSPF as application layer in the link-layer group

Docstrings and comments only; AST identical to main once stripped.
@JarryShaw JarryShaw added docs Pull requests that change documentation only (docs: subject prefix) 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, 6f97862dd, 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

Copy link
Copy Markdown
Owner Author

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

  • AST guard: all 10 files are identical to origin/main once bare strings are stripped.
  • null.py: schema_final generates an __init__ only when the class does not define one. Probed: NoPayload({}) raises TypeError.
  • ethernet.py: _lookup_next_layer writes a resolved ModuleDescriptor back into the registry, and misses are not recorded.
  • mh.py:
    • The Pad1 length is set to 0 in post_process.
    • The IGMP/MLD values match RFC 7028 §8 and §5.1.2, fetched.
  • ipv4.py: the Quick-Start options inherit type and length from Option rather than re-declaring them.
  • tcp.py: ConditionalField is checked by type in both pack and unpack.
  • Cross-references: in a full Sphinx -n build, the repointed Protocol._lookup_registry and EnumLookup targets resolve with no warnings.
  • Tests: test_docstring_contract, three schema modules and all 22 tests/project modules pass, one module per process.

Non-blocking: the OSPF comment is true but sits under the # Link Layer Protocols heading of __all__. The real fix is to move the names into the application group. That is a code edit, so it will be filed as a follow-up rather than added to this prose-only PR.

UNVERIFIED: the rendered HTML (checked with the dummy builder only), and there is no base-tree Sphinx diff.

@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
@JarryShaw
JarryShaw merged commit 446a6ff into main Oct 6, 2026
63 of 65 checks passed
@JarryShaw
JarryShaw deleted the docs/719-r2-schema branch October 6, 2026 17:37
@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