Repository navigation
Conversation
Round 2 of the #1103 repeating #719-style sweep. Where round 1 (#2408) was scoped to one recent PR wave, this round covers the ground that wave did not touch: all of pcapkit/ outside const/ and vendor/, docs/source/, the root documents, util/, examples/ and tests/. Prose only -- docstrings, comments and .rst/.md text -- with no behaviour change and no renames. Cross-references converged on the class that defines the member: * every `pcapkit.protocols.protocol.Protocol.<member>` target under pcapkit/ now names `ProtocolBase`, which is where all of them are defined -- `Protocol` declares only `__init_subclass__`. This finishes the direction round 1 took in the `register`/`register_protocol` docs; the tree had been split roughly evenly, with the same members (`alias`, `unpack`, `from_data`, `analyze`, `expand_comp`, `_lookup_registry`) spelled both ways. * display text that already read `ProtocolBase.X` over a `Protocol.X` target is no longer a mismatch (utilities/chardet.py, protocols/data/data.py, internet/ipv6_ext.py), and `pcapkit.protocols.data.protocol.Protocol.packet` -- a different class -- is left alone. Prose corrected where it did not match the code: * schema/internet/hip.py's `parameter_padding_len` named every parameter in the module as its caller; `LocatorSetParameter` uses `locator_set_padding_len`, which the field's own doc-comment already said. * conventions/utilities-or-corekit.rst omitted timestamp conversion from what pcapkit.utilities holds, which utilities/__init__.py's own docstring lists. * CONTRIBUTING.md said no red linter fails a pull request; lint.yml's actionlint step carries no `continue-on-error` and exits on its status, as that workflow's own header records. * the class-hierarchy graph in protocols/index.rst drew HIP single-parented; it derives from both `IPv6_Ext` and `Internet`, the only internet class that does, and the graph records AH's and ESP's second parent the same way. Before/after narration replaced with present tense: * internet/mh.py's two `EnumLookup` notes narrated a rejected `get` override ("deleted, not widened to accept `default`"); the reason it would be redundant is kept, the history dropped. * data/internet/hip.py's `cert_group` repeated the Diffie-Hellman-registry history already carried by its `.. versionchanged:: 1.5.0` note. * tests/project/test_annotation_names.py no longer dates the trap in months. Counts of codebase items dropped, per the ruling on #1103: * tests/const/test_const_enum_builtin_parity.py cited 113 of 117 const modules; the tree holds 124, and neither figure is asserted anywhere in the module. AST guard (ast.dump via util/vendor_code_changed.py's `code_of`, docstrings and bare string statements stripped) confirms all 40 touched .py files are prose-only against 27c797d; no added line exceeds the 120-column limit.
|
Coverage: 91.73% (unit tier, Python 3.15,
Per-file detail: the |
|
Review: NEEDS CHANGES at Core claim independently verified: Two fixes needed:
Posting the docs-structure question (whether |
|
Closing — the owner ruled on #1103 that |
Please follow the guide below
You will be asked some questions, please read them carefully and answer honestly
Put an
xinto all the boxes [ ] relevant to your pull request (like that [x])Use Preview tab to see how your pull request will actually look like
Searched for similar pull requests
Followed the coding style (
make pylint,make mypy,make isort)make testpasses, and a test case covers the changeAdded a changelog entry under
docs/source/changelog/and regeneratedCHANGELOG.md, if the change is user-visible — N/A, prose only and not user-visible, as for round 1 (docs: #719-style prose sweep after the #1090-1102 defect wave #2408), which carries no entry eitherWhat is the purpose of your pull request?
Tick the commit type your subject line carries.
fix— corrects a defectfeat— adds a featureperf— changes performance, not behaviourrefactor— changes neither behaviour nor performancetest— tests onlydocs— documentation onlyci— workflows or build toolingrelease— bumps the version or rolls up a distributionchore— anything elseDescription of your pull request and other information
Round 2 of the #1103 repeating #719-style sweep. Round 1 (#2408) was scoped to one recent PR wave; this round covers the ground that wave did not touch — all of
pcapkit/outsideconst/andvendor/,docs/source/, the root documents,util/,examples/andtests/. 43 files, prose only (docstrings, comments,.rst/.mdtext), no behaviour change, no renames.1. Cross-references converged on the defining class
Every
pcapkit.protocols.protocol.Protocol.<member>target underpcapkit/now namesProtocolBase.Protocoldeclares only__init_subclass__;payload,unpack,alias,info,read,pack,analyze,_file,__header__,_decode_next_layer,_import_next_layerand the rest are all defined onProtocolBase. This finishes the direction round 1 took in theregister/register_protocoldocs — before it the tree was split roughly evenly, with the same members (alias,unpack,from_data,analyze,expand_comp,_lookup_registry) spelled both ways in different modules.Three places already read
ProtocolBase.Xin their display text over aProtocol.Xtarget, which is the mismatch round 1 fixed inlink.pyandtransport.py:utilities/chardet.py,protocols/data/data.py,internet/ipv6_ext.py.pcapkit.protocols.data.protocol.Protocol.packetis a different class and is left alone.One thing to rule on:
docs/source/andtests/still spell it the other way — 26 occurrences across 12.rstpages and 5 test modules. That is not accidental:docs/source/pcapkit/protocols/protocol.rstrenders these members under theProtocolentry and itsProtocolBasesection links on to them as:attr:~Protocol.name``. So both targets resolve, and finishing the conversion there means deciding where the members should render, which is a structural docs call rather than a sweep one. Flagged rather than taken.2. Prose corrected against the code
schema/internet/hip.py—parameter_padding_lenclaimed every parameter schema in the module as its caller.LocatorSetParameteruseslocator_set_padding_len, which that field's own doc-comment already said.conventions/utilities-or-corekit.rst— omitted timestamp conversion from whatpcapkit.utilitiesholds, whichpcapkit/utilities/__init__.py's own docstring lists andpcapkit/utilities/datetime.pyprovides.CONTRIBUTING.md— said a red linter will not fail a pull request. TheLintjob'sactionlintstep carries nocontinue-on-errorand ends onexit "$status", aslint.yml's own header records ("Every step but actionlint is advisory").protocols/index.rst— the class-hierarchy graph drewHIPsingle-parented. It isclass HIP(IPv6_Ext[...], Internet[...], ...), the only class underprotocols/internet/with a secondInternetbase, and the graph recordsAH's andESP's second parent the same way.3. Before/after narration → present tense
internet/mh.py— twoEnumLookupnotes narrated a rejectedgetoverride ("the override was deleted, not widened to acceptdefault"). The reason it would be redundant is kept; the history is dropped.data/internet/hip.py—cert_group's doc-comment repeated the Diffie-Hellman-registry history that its own.. versionchanged:: 1.5.0note already carries.tests/project/test_annotation_names.py— no longer dates the trap in months.4. Counts of codebase items dropped (the ruling on #1103)
tests/const/test_const_enum_builtin_parity.pycited "113 of the 117 modules" underpcapkit/const/; the tree holds 124, and neither figure is asserted or computed anywhere in the module. The decision and its reasoning are kept.Defects found while sweeping, reported rather than fixed
These are code, not prose, and are left for their own issues:
FTP.make'scodebranch is unguarded (protocols/application/ftp.py).code_val = int(code)has no type guard, where the siblingcmmdbranch got one on fix(ftp): make raises a bare AttributeError for a command that is not str or bytes #2290 andargson fix(application): FTP.make's args rejects bytearray/int with AttributeError instead of ProtocolError #2410. Measured on this branch:FTP.make(obj, code='abc')raises a bareValueError: invalid literal for int() with base 10, not theProtocolErrortheRaises:section promises;code=Trueis accepted and writesb'001'although thecmmdguard rejects aboolexplicitly;code=3.7is accepted and writesb'003'. (Repro needsFTP.makeon an uninitialised instance —FTP().make(code=...)takes theelsearm and raisesProtocolError: FTP: invalid packet type.)Deferred.__init__'sprotoannotation is too narrow (foundation/reassembly/data/data.py). It isproto: 'TransType'withanalyze: 'Callable[[TransType, bytes], ProtocolBase]', butTCP.submitpasses(bufid[1], bufid[3])— a port pair — becauseTransport.analyzeoverrides the signature toports: tuple[int, int]. Behaviour is correct; the annotation and theproto: Payload protocol type.docstring describe only the IP case.test_actionlint_in_lint_1967.py's stated reason does not justify its floor. The docstring says 1.7.12 is needed because "the workflows use YAML anchors, which 1.7.7 cannot read", butlint.yml's own step comment says anchors need 1.7.10 or later.MINIMUM = (1, 7, 12)is what the test asserts, so the real reason for 1.7.12 is recorded nowhere. Not fixed because inventing a reason would be worse.test_file_size_caps_1969.pyscans one of two callers.WORKFLOW = 'unit-tests.yml', butgrep -rln cap_file_size .github/workflows/returnsunit-tests.ymlandexperimental.yml, so an uncapped pytest step added to experimental's Linux job fails nothing. (The Windows leg's uncapped step is deliberate —resourceis unavailable there.)ext.rstand the conventions pages disagree on HIP's classification.ext.rst's shipped-classes table listsHIPas a plain Internet-layer row besideIPXwhileMHsits under IPv6 Extension Header, though both derive fromIPv6_Ext. Which side of HIP's dual nature that grid should show is a presentation call.Scope not reached
Honest about the edges, so a round 3 knows where to start:
docs/source/contributing/workflows.rst(2055 lines) andreleasing.rst(613) got the mechanical passes but no line-by-line read; the per-protocol API pages underdocs/source/pcapkit/were covered by the structural sweeps only; the repetitive per-field#:comments in the largestschema/anddata/modules were sampled rather than read exhaustively; and the bodies of the largesttests/project/modules (test_release_gates.py,test_ci_timing.py,test_conventions_doc_claims.py) were swept mechanically rather than read.Test plan
ast.dumpviautil/vendor_code_changed.py'scode_of, with docstrings and bare string statements stripped: all 40 touched.pyfiles are prose-only against27c797d35, 0 differences. The 3 remaining files areCONTRIBUTING.mdand two.rstpages.--max-line-length;format/docstylefindings on the touched files are all pre-existing lines this PR does not touch.tests/test_docstring_contract.py,tests/test_base_class_contract.py,tests/project/test_conventions_doc_claims.py,test_documentation_claims.py,test_docs_issue_pr_roles_1103_unit.py,test_issue_refs_in_comments.py,test_annotation_names.py,test_docs_glossary_1949_unit.py,test_docs_internal_refs_1940_unit.py,test_docs_internal_refs_1947_unit.py,test_docs_ambiguous_xrefs_1775_unit.py,test_docs_unresolvable_roles_1919_unit.py,test_docs_missing_directives_2234_unit.py,test_docs_warns_xrefs_2094_unit.py,test_pr_template_commit_types_1102_unit.py,test_python_versions_1660.py,test_utilities_import_boundary_1735_unit.py,test_no_moved_name_re_exports_1744_unit.py,tests/const/test_const_enum_builtin_parity.py— 186 passed, 22 skipped, 1604 subtests passed.make test's exclusions (--ignore=tests/integration --ignore-glob='*_runtime.py' --ignore-glob='*_regression.py') —tests/protocols,tests/foundation,tests/corekit,tests/utilities,tests/interface: 3332 passed, 13 skipped, 29807 subtests passed, 0 failures.One thing worth recording, since it looks alarming out of context
Running those same directories without the
make testexclusions gives 89 failures here. They are all in the fixture-dependent*_runtime.pytier, which reads generated captures:tests/foundation/traceflow/test_traceflow_snapped_capture_runtime.py's own docstring says "The module reads the generatedtcp.pcap, so it belongs to the fixture-dependent tier", and this checkout'sexamples/captures/holds onlyin.pcapanddhcp.pcapngbecausemake sampleshas not been run.make testexcludes that tier by design,make test-alldepends onsamples, and main's Unit Tests run is green at27c797d35— this PR's base. Nothing to do with this change, which the AST guard shows cannot alter behaviour.