Skip to content

docs: #1103 round 9 -- finish the Protocol/ProtocolBase sweep, add const as a commit-type - #2431

Merged
JarryShaw merged 1 commit into
mainfrom
docs/1103-round9-protocolbase-const
Oct 29, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
docs/1103-round9-protocolbase-const

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 29, 2026 •

Copy link
Copy Markdown
Owner

Summary

Round 9 of the whole-repo prose sweep (#1103) -- a scoped pass over exactly the 4 items left outstanding from round 8 (#2428), not a fresh whole-repo sweep.

No executable code changed -- every .py edit is inside a docstring. Verified with an AST-structure diff (docstrings stripped) against origin/main for both touched files: identical.

Test plan

  • Searched for similar pull requests
  • make pylint/make mypy/make isort -- not run (prose-only change, no code touched)
  • make test equivalent: ran tests/project/ in full on this branch and on a clean main checkout under the same toolchain. Failure/error sets are byte-for-byte identical (234 failed/11 errors, all pre-existing and environmental -- subprocess tests that can't resolve util/_actions in this sandbox -- 2010 passed either way). Also built docs with the pinned toolchain (Sphinx==9.2.0) on this branch and on main: both produce exactly the same 1 pre-existing, unrelated warning (TransType cross-reference ambiguity).
  • Changelog entry: N/A -- centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

What is the purpose of your pull request?

  • docs -- documentation only

Refs #1103.

@github-actions github-actions Bot added this to the PyPCAPKit v1.5 milestone Oct 29, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 29, 2026
@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 review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 29, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Review: NEEDS CHANGES at 57349ef91 (Sonnet). One real finding, text-only:

tests/project/test_conventions_doc_claims.py:1319-1320 still says "const is a type-ish label with no tickbox, so this is a one-way check by design" — false now that this PR adds the tickbox. The test still passes (it's a comment, not an assertion), but the sentence needs rewording or dropping.

Non-blocking: the PR body's dd036a52d precedent for const: as a commit-type prefix only exists on a PR ref (pr/503), not on origin/main (which merged as fix(const): … labelled fix) — origin/main has 0 const-typed subjects today, so "those are the ones in use" isn't literally true yet. Worth softening the claim, not blocking since the owner already approved adding const regardless of prior usage. Template puts const after the catch-all chore — cosmetic, your call.

Everything else verified independently: diff scoped to exactly the 4 claimed files (conf.py genuinely untouched), protocol.py/transport.py flips complete and correct (14→3 remaining dotted references, all in documented carve-outs), register's Raises: clause and its guard test untouched, the PR-template test genuinely parses dynamically (reverting either const edit alone fails it), both Sphinx 9.1 comments re-verified accurate at 9.2.0 via isolated probes against a 9.1.0 control, AST guard clean, docs build HTML diff limited to the 12 expected retargeted anchors, no scope creep.

@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 94.44% (unit tier, Python 3.15, da480c468, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19609 133 2620 120 98.84%
pcapkit/corekit 2605 74 818 37 96.52%
pcapkit/dumpkit 282 3 102 3 98.44%
pcapkit/foundation 3064 24 1068 26 98.79%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17593 193 4678 179 98.32%
pcapkit/toolkit 924 36 334 10 95.55%
pcapkit/utilities 502 7 144 5 98.14%
pcapkit/vendor 4449 1854 1020 167 55.11%

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

…nst as a commit-type

* pcapkit/protocols/protocol.py: flip the 11 remaining unqualified
  `ProtocolBase.<member>` cross-references to `Protocol.<member>`, display and
  target both, matching the mechanical flip #2420/#2422/#2428 already did for
  37+ other sites. The `__init_subclass__` sites and `register`'s `Raises:`
  clause are untouched, per the existing #2316/#1016 carve-out.
* pcapkit/protocols/transport/transport.py: same flip for the
  `ProtocolBase._lookup_registry` reference #2421 added after #2422's sweep
  had already run, so it was left behind.
* CONTRIBUTING.md, .github/PULL_REQUEST_TEMPLATE.md: add `const` to the valid
  commit-type list and PR template checkbox. process.rst already documented
  `const` as a Type label; CONTRIBUTING.md and the template had not caught up.
  `tests/project/test_pr_template_commit_types_1102_unit.py` derives its
  expected set from CONTRIBUTING.md, so no test change was needed.
* tests/project/test_conventions_doc_claims.py: drop the now-false "const has
  no tickbox" example from
  test_the_page_names_every_commit_type_the_template_ticks's docstring --
  the template gained one above -- and restate why the check is one-way
  without relying on that example.
* docs/source/conf.py's two "Sphinx 9.1" comments were re-verified against
  the pinned toolchain (Sphinx 9.2.0, per docs/requirements.txt) by instrumenting
  each code path directly: autodoc still hands `is_enum_member` the plain int
  for an enum member, and still prefers a pass-through `__new__` over `__init__`
  for a class signature. Both comments are accurate at 9.2.0 and left as is.

Prose-only: an AST-structure diff (docstrings stripped) against origin/main is
identical for both touched .py files. `make -C docs html` under the pinned
toolchain is warning-for-warning identical to a main build (1 pre-existing,
unrelated `TransType` ambiguity). `tests/project/` is failure-for-failure
identical to main (234 failed/11 errors, all pre-existing and environmental,
unrelated to this change; 2010 passed either way).
@JarryShaw
JarryShaw force-pushed the docs/1103-round9-protocolbase-const branch from 57349ef to da480c4 Compare October 29, 2026 15:11
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 29, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Review: GOOD TO GO at da480c468. Verified directly (main thread). The stale const-specific example is dropped from test_conventions_doc_claims.py's docstring, reworded generically — confirmed correct, test passes clean (34 passed/193 subtests). Diff scoped to exactly the expected 5 files.

Both non-blocking items left as-is — both were explicitly optional in the last round's verdict.

@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 29, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Correction to my own GOOD TO GO comment: I said both non-blocking items were left as-is. That's wrong for one of them — the worker did soften the dd036a52d precedent claim in the PR body (now reads "no precedent on main today"). Only the checkbox ordering was genuinely left alone. Verdict stands (GOOD TO GO), independently re-confirmed by a second review pass since.

@JarryShaw
JarryShaw merged commit 857bd52 into main Oct 29, 2026
65 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 29, 2026
@JarryShaw
JarryShaw deleted the docs/1103-round9-protocolbase-const branch October 29, 2026 16:15
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 29, 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