Skip to content

docs(const,vendor): state the generated files' rulings instead of quoting them - #996

Merged
JarryShaw merged 1 commit into
mainfrom
docs/987-const-vendor-rulings
Oct 2, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
docs/987-const-vendor-rulings

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 2, 2026 •

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

Closes #995.


Description of your pull request and other information

The last tranche of #987, plus #995. These files are generated, so every change is made twice — in the vendor template and identically in the const file it renders. The template holds the prose with {NAME}/{DOCS} placeholders, so the next crawler run reproduces it. No crawler was run; they need the network and the vendor extra.

Scope. 22 italic-quoted spans across 7 files, of which 16 are rulings and are converted. Six are not and stay — all of them RFC 5797 and RFC 2389 text on FEAT-code case sensitivity. The italic pattern also missed 14 straight-quoted spans in the apptype pair, 7 per side, most wrapping a backticked vertical bar; those are converted too.

#877 was revised twice, and this pull request's first revision rendered the middle one. The final ruling is RFC-directed: an enum treats its values as case-insensitive where the RFC states they are, and as case-sensitive otherwise. The earlier form, which also allowed a fold where it logically made sense, is out of both halves of the ftp pair.

#860's ruling is narrowed to what it actually says. The prose called it the ruling for the whole family. The comment reads "for all three" and names FEATCode, Command and Method; AppType is never named in it, and the AppType work is #874 under #860. So the prose now says the family follows a ruling given for those three. One flattened conditional is restored — #860's FEAT-value question was answered conditionally, on whether it matched the approach already in use — and one lost clause: the #921 exception ruling also says the exception comes from pcapkit.utilities.exceptions rather than being a builtin.

#995: vendor/ipx/socket.py credited a ruling with "a real ownership fact", which appears in no maintainer comment. The real reason, #847 at 13:15:36Z, is that a proprietary protocol may expose no name of its own. That #: block is module documentation for UNASSIGNED_RANGE_NAMES and is not emitted into the const file, so only the template changed.

Prose only, proven against the one risk that matters in a generated file. A sha256 over every name and value is byte-identical between the two trees for the seven enums the three touched const files define — 127 members, 126 iterable. AppType is a runtime registry and contributes none of them, so its generated reStructuredText rows are checked separately: identical at 603 distinct of 707. Token sequences match per file with comments and NL dropped, masking FSTRING_MIDDLE as well as STRING, since the templates are f-strings and their prose tokenises as the former. The over-95 line multiset is unchanged in every file, and six wrapping warts the re-flow introduced — one split inline literal and five orphan tails — are closed.

Two things left for later, both outside this tranche: the same phrase survives in tests/vendor/test_ipx_socket_unit.py and in tests/const/test_const_enum_no_mint.py, and a further set of italic spans under pcapkit/protocols/ were not examined — 25 by my own multi-line-aware extraction against the worker's 26, a one-span discrepancy the follow-up reconciles. 23 of the 25 wrap across lines, which is why a single-line regex finds only two, and every one sampled is RFC text rather than a ruling.

@JarryShaw JarryShaw added const Regenerated IANA or vendor constant tables; members keep their numeric values 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 labels Oct 2, 2026
@JarryShaw
JarryShaw force-pushed the docs/987-const-vendor-rulings branch from 6709fb3 to 56bd6cf Compare October 2, 2026 20:30
@JarryShaw

Copy link
Copy Markdown
Owner Author

NEEDS CHANGES at 6709fb308, fixed and pushed as 56bd6cfa0 — opus cross-review. One blocking defect, and it is this pull request's own subject turned back on it.

#877 revised twice, and the prose stated the middle revision. The passage kept case-insensitivity to selected registries "where it logically makes sense or where the RFC itself treats the names as case-insensitive" — a faithful rendering of the 02:30:58Z redaction, except that the thread runs on. At 02:48:29Z the maintainer reissues it RFC-directed: case-insensitive where the RFC states the values are, case-sensitive otherwise, recorded as final at 02:51:38Z. That limb is not in the ruling, so it is out of both halves of the pair.

Two cosmetic items alongside it — the re-flow had wrapped a TransportProtocol.tcp | TransportProtocol.udp literal that the base had whole, and left orphan tails. Six in all, closed.

Four corrections to my own account, two of them to things I published in the commit message.

  • 22 italic spans, not the 23 I wrote. My count folded in vendor/default.py's span, which is straight-quoted.
  • Six stay untouched, not seven, and all six are RFC text. The Wikimedia 403 body I listed as the seventh is straight-quoted too.
  • My member hash was mis-scoped: it covered Socket from the untouched const/ipx/socket.py and omitted FEATCode, CommandType, ConformanceRequirement and AppType, all of which this change does touch.
  • The author's inventory was not self-contradictory as I called it — 22 base, 6 staying, 16 converted is consistent, and I was wrong to resolve it by discarding the total.

Re-verified at 56bd6cfa0: sha256 over every name and value byte-identical for the seven enums the three touched const files define (127 members, 126 iterable); generated rST rows identical at 603 distinct of 707; every changed line is prose — 174/180 comment, 98/94 docstring, 9/7 #: — with zero assignments or member definitions; over-95 line multiset unchanged in all seven files. Round-2 review dispatched on a different model.

One item this change does not touch, for the record: const/ftp/command.py:101 calls FEATCode's fold "one of the few case-insensitive overrides the ruling on #877 allows". That wording is on main already and reads from the two-limb framing, though the sentence supplies the RFC basis immediately after. It belongs to #987's remaining sweep, not here.

…ting them

The last tranche of #987, plus #995. These files are generated, so every change
is made twice: in the vendor template and identically in the const file it
renders. The template holds the prose verbatim with {NAME}/{DOCS} placeholders,
so the next crawler run reproduces it. No crawler was run.

Scope: 22 italic-quoted spans across 7 files, of which 16 are rulings and
converted. Six are not and stay -- all of them RFC 5797 and RFC 2389 text on
FEAT-code case sensitivity. The italic pattern also missed 14 straight-quoted
spans in the apptype pair, 7 per side, most wrapping a backticked vertical bar;
those are converted too.

- #877 was revised twice and the prose stated the middle revision. The final
  ruling is RFC-directed: an enum treats its values as case-insensitive where
  the RFC states they are, and as case-sensitive otherwise. The earlier form,
  which also allowed a fold where it logically made sense, is out.
- #860's ruling is narrowed to what it says. The prose called it the ruling for
  the whole family, but the comment reads "for all three" and names FEATCode,
  Command and Method. AppType is never named in it, and the AppType work is
  #874 under #860, so the prose now says the family follows a ruling given for
  those three rather than that it was given for the family.
- #860's FEAT-value question was answered conditionally, on whether it matched
  the approach already in use; that conditional is restored.
- The #921 exception ruling had lost its second clause -- that the exception
  comes from pcapkit.utilities.exceptions rather than being a builtin.
- #995: vendor/ipx/socket.py credited a ruling with "a real ownership fact",
  which is in no maintainer comment. The real reason, #847 at 13:15:36Z, is
  that a proprietary protocol may expose no name of its own. That comment is
  module documentation for UNASSIGNED_RANGE_NAMES and is not emitted into the
  const file, so only the template changed.
- The exception ruling was credited to issue #923, which carries no maintainer
  comment at all -- its own body attributes the ruling to the review of #877's
  implementation. So the prose now credits the ruling to that review and #923
  with the implementation, keeping the citation in issue form as
  docs/source/contributing/conventions/documentation.rst requires.
- Re-flowing wrapped one inline literal that the base had whole, and left five
  orphan tails. All six are closed.

Prose only, and proven against the one risk that matters in a generated file.
Importing both trees gives a byte-identical sha256 over every name and value for
the seven enums the three touched const files define -- 127 members, 126
iterable -- and the generated rST table rows are identical at 603 distinct of
707. Token sequences match per file with comments and NL dropped, masking
FSTRING_MIDDLE as well as STRING since the templates are f-strings and their
prose tokenises as the former. The over-95 line set is unchanged in every file.

Closes #995.
@JarryShaw
JarryShaw force-pushed the docs/987-const-vendor-rulings branch from 56bd6cf to 95c1182 Compare October 2, 2026 20:40
@JarryShaw

Copy link
Copy Markdown
Owner Author

Round 2: GOOD TO GO at 56bd6cfa0 — sonnet cross-review, a different model from the author. It read #877's thread independently (38 comments, --paginate) and matched the revision sequence, confirming the new wording renders the 02:48:29Z verdict rather than the 02:30:58Z redaction. Pairs in step; token-identical with strings and f-strings masked; 127 members / 126 iterable with identical per-class hashes; no over-long line, no split literal, and no role with a space inside its target. Where our numbers differ they are pattern-dependent and agree before-to-after either way — it counted 601 distinct rST rows to my 603.

Its one nit was this pull request's own defect class, so it is fixed rather than shipped. New head 95c11824e. The prose credited "issue #923's ruling". #923 carries no maintainer comment at all — its four comments are agent reports, and its own body opens by attributing the ruling to #921's review. The ruling is at #921, 2026-09-29T15:24:06Z.

But #921 is a pull request, and docs/source/contributing/conventions/documentation.rst:196 says to name the issue and not the pull request, with the exemption at :203 covering only the changelog and tests/. The resolution is the form these two files already carry seven times — "a ruling given in review of the work for #808". PR #921 is the work for issue #877, so the sentence now reads "the ruling given in review of the work for #877, implemented in GitHub issue #923". No bare pull-request number survives on any changed line.

Re-verified at 95c11824e: all seven files token-identical to main with STRING and every FSTRING_* type masked and comments dropped; over-95 multiset unchanged with zero new long lines; generated rST rows unchanged at 603 distinct of 707; and the reworded paragraph byte-identical between the const and vendor halves. Round-3 verdict on the new head to follow.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Round 3: GOOD TO GO at 95c11824e — sonnet, scoped to the delta. The two-line citation change is the only difference from 56bd6cfa0, which round 2 had already cleared.

It settled the one thing I had flagged as unchecked. Every #NNN on an added line is an issue, #759 included — I re-derived that independently: #759, #775, #808, #841, #860, #877 and #923 all return pull_request: null, and no pull-request number (#847, #874, #921) appears on any added line. It also confirmed the new citation is sound on its own terms: PR #921's body opens by describing itself as phase 2 of #877's ruling, so "given in review of the work for #877" is accurate rather than merely convenient, and it had no better wording to propose.

Re-derived at this head rather than carried over: all seven files token-identical to main with STRING and every FSTRING_* type masked and comments dropped; AST equal with docstrings blanked; member sha256 byte-identical across the seven enums (127 members, 126 iterable); generated rST rows unchanged at 603 distinct of 707; over-95 multiset unchanged with no new long line; no orphan at the ~80-column band; no split inline literal and no space inside a role target.

It declared two checks UNVERIFIED and was right to — it did not re-run the import hash or the row extraction, neither of which a change confined to one docstring paragraph can reach. I ran both myself at 95c11824e, as above.

Label is now review: good-to-go. Not yet ready to merge: CI at this head is 9 passed, 3 skipped, 58 still in flight, zero failures. The verdict is what the label tracks; the in-flight count is what gates calling it ready.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 2, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

One citation note, and a deliberate decision not to amend for it. The commit message and this description cite "#847 at 13:15:36Z" as the source of the ownership ruling. #847 is a pull request (fix(ipx): order Socket._missing_ range branches narrowest-first, closing #841), and a stronger citation exists: #775 carries the same ruling directly, at 2026-09-27T19:53:45Z, and #775 is an issue.

The shipped prose is unaffected — pcapkit/vendor/ipx/socket.py:257-259 cites #775 and #841, not #847, so the diff holds the issue-form rule as written. The weaker citation is confined to the commit message and this description, where a pull-request number is a pointer to a change rather than documentation prose — the same basis on which the changelog cites pull requests by design, and on which this message also names #874.

So I am leaving it: amending would invalidate a verdict already given at 95c11824e and restart seventy-odd checks for no change to any shipped line. Recorded here instead so the better citation is on the record rather than lost.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Ready to merge at 95c11824e — closing the "not yet" on my round-3 comment. CI finished clean: 69 success, 3 skipped, 0 in flight, 0 failures across 71 CheckRuns, and mergeStateStatus is CLEAN against main at 0ac0c89ea. Three cross-review rounds on a model other than the author's: NEEDS CHANGES at 6709fb308, then GOOD TO GO at 56bd6cfa0 and again at this head.

One commit, prose only, on top of the current main. Closes #995.

Unpublished decisions are yours, and I have not merged. Two things queued behind this one, both deliberately: #987's last residual is fixed and verified but unpushed, because main's ruleset has strict: true and any push would flip this pull request to BEHIND and restart seventy-odd checks; and #989 stays blocked until #987 closes, to keep two branches out of the same docstrings.

@JarryShaw
JarryShaw merged commit 9644624 into main Oct 2, 2026
73 checks passed
@JarryShaw
JarryShaw deleted the docs/987-const-vendor-rulings branch October 2, 2026 21:10
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

const Regenerated IANA or vendor constant tables; members keep their numeric values docs Pull requests that change documentation only (docs: subject prefix)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs(vendor): a ruling is credited with wording the maintainer never used

1 participant