Skip to content

docs(readme): trim to a landing page and convert to Markdown - #619

Merged
JarryShaw merged 2 commits into
mainfrom
docs/readme-landing-page-markdown
Sep 22, 2026
Merged

JarryShaw merged 2 commits into
mainfrom
docs/readme-landing-page-markdown

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Sep 22, 2026

Copy link
Copy Markdown
Owner

Requested by the project owner, in their words: "for README specifically, we might want to remove some of the technical details from it - and maybe change back to Markdown format."

Both halves are done, and the references to the renamed file were updated with it — a change that renames a file owns the references to it.

What the README became

424 lines / 17,951 bytes → 103 lines / 4,548 bytes.

What stays is what a reader arriving from PyPI or a search result actually needs: what the library is, why it exists rather than Scapy/DPKT/PyShark, how to install it, a worked example, and where the real documentation is.

The worked example is new — the old README had no usage example at all, which is an odd gap for a landing page. Every value in it was measured against this branch using the committed examples/captures/in.pcap, not written from plausibility. (The first draft guessed len(flows.trace) was 0; it is 3. A README example is the first thing a reader tries and the only thing they can check without reading source, so a wrong number there is worse than no example.)

Nothing was deleted that existed only in the README

Every block removed was checked against docs/source/ first.

Already there, in a fuller form — now linked instead of restated:

Block removed Where it already lived
Module Structure docs/source/index.rst:60-103 (also pcapkit/__init__.py's docstring)
Engine Comparison docs/source/index.rst:105-144
Engine support by Python version docs/source/index.rst:146-172
Test Environment docs/source/index.rst:174-184
Test Results (+ footnotes 1-4) docs/source/index.rst:186-217
Installation Notes docs/source/index.rst:291-312
Engine prerequisites docs/source/index.rst:314-409, plus the gap tables in docs/source/pcapkit/foundation/engines/index.rst:72-150

Existed nowhere else — moved, not dropped:

  • The Testing section is now docs/source/testing.rst, a new page registered in the index.rst toctree. There was no testing page in the docs at all; pep.rst:586-599 covered the untracked-fixtures fact and make samples, but make test-all, the tshark-is-only-for-PyShark note, the examples/legacy_smoke/ fixture path and the examples/generators/options.py round-trip mechanism had no copy anywhere else. make test-all in particular appeared only in the Makefile and this README.
  • The pipenv / make setup local development block (the ~/Library/Caches permission issue, the Homebrew libxml2/libxslt issue, and the PIPENV_VENV_IN_PROJECT=1 … invocation) joined the Installation section of docs/source/index.rst. It was not in the docs, and not in CONTRIBUTING.md either.

The convention this deliberately crosses

This repo's standing rule is that documentation is .rst, never .md, with the generated root CHANGELOG.md as the one exception. The owner is overriding that for the README specifically, so please do not "fix" this back.

The rationale, for the record: the README is the one documentation file whose renderers are GitHub and PyPI rather than Sphinx. That is exactly why it has been a recurring hazard — 1.5.0b1 half-shipped because twine check found a Sphinx-only :mod: role in README.rst, and the whole examples/benchmark/report.py toolchain carries defensive machinery to keep Sphinx roles out of the emitted tables. A Markdown README removes the class of failure where RST-that-Sphinx-accepts is RST-that-PyPI-rejects.

No other .rst file was converted.

The references to the renamed file

examples/benchmark/Dockerfile copied README.rst by literal name, with no glob and no .dockerignore in the repository, so the layer failed outright and took make bench, make bench-quick and run.sh with it. Now README.md. Verified both ways with a FROM scratch build against the same context run.sh uses (REPO_ROOT), which is instant and needs no image pull:

===== fixed line (README.md) =====
sha256:dfb4549dd4a8a25c5db79a0ab3468503c4f65a33e871c7498d3b7eead1e54d02
===== old line (README.rst), expected to fail =====
ERROR: failed to solve: failed to compute cache key: failed to calculate
checksum of ref ...: "/README.rst": not found

The benchmark harness prose named the root README as where its generated tables land. That was stale twice over — wrong extension and wrong destination, since Test Results and Test Environment are two of the sections that moved to docs/source/index.rst. Rather than correct the extension and leave them pointing at a file that no longer holds those tables, the destination was fixed properly across all of it: the Makefile comment, report.py (9 sites), test_harness.py (7), run.sh (2) and examples/benchmark/README.rst (5).

One of those was not a stale path but an inverted reason. report.py and the suite's README both justified emitting docutils-safe RST on the grounds that GitHub renders the README with docutils, where a :mod: role is a visible error block. The destination is now a Sphinx-rendered page, where a Sphinx role would in fact survive. The generator's behaviour is unchanged and its tests still assert docutils-cleanliness — but the prose now says what is true: staying docutils-clean is a conservative choice that keeps a table pasteable into any reStructuredText a plain docutils reader will see, rather than a hard requirement. Relaxing the constraint is a design call for that tree's owner and was not taken here.

examples/benchmark/benchmark.py:25 still says README.rst, and is correct. It was reported to me as stale; it is not. It refers to the benchmark suite's own README.rst in the same directory, whose "Departures from the legacy methodology" section is at line 247. Left alone.

Also left alone: CONTRIBUTING.md:9, handled separately by dropping the extension, and tests/project/test_changelog_md.py:432, a string fixture describing the historical 1.5.0b1 failure.

Verification

twine check --strict on a real build, re-run after the rebase onto 545b174bf:

Checking /tmp/dist_final/pypcapkit-1.5.0b4-py3-none-any.whl: PASSED
Checking /tmp/dist_final/pypcapkit-1.5.0b4.tar.gz: PASSED

Also confirmed:

  • README.md present at the sdist root, no README.rstinclude README.md in MANIFEST.in is what does it. global-include *.rst no longer matches the README, so that line went from belt-and-braces to load-bearing, and the comment above it now says so.
  • Wheel metadata reads Description-Content-Type: text/markdown with the 4,548-character body.
  • 167 tests pass: tests/project/test_setup.py (4), tests/project/test_changelog_md.py (47 + 37 subtests), examples/benchmark/test_harness.py (116). The setup tests execute setup.py top-to-bottom, so they would catch a stale README.rst reference; they are filename-agnostic and assert only that the description contains PyPCAPKit.
  • python util/changelog_md.py --check exits 0. The rebase conflicted on both changelog files, as expected with siblings landing; the .rst was resolved keeping all four of 545b174bf's bullets with mine last, and CHANGELOG.md was regenerated rather than hand-merged.
  • Scoped Sphinx build of just index.rst + testing.rst: build succeeded, testing.html renders, and no "document isn't included in any toctree" warning, so the toctree registration is good.
  • All 16 links in the new README resolved: 2 repo-relative (both present), 8 docs-site links whose target pages and anchors (#installation, #module-structure, #engine-comparison) were confirmed in the generated HTML, and 6 external links carried over unchanged.

Two things found and deliberately not changed

No licence section was added to the README. The old one had none, and the licence question is open, so this keeps the status quo rather than asserting something that may be about to change.

A possible hang in the public API, unrelated to this change and being filed separately. no_eof=True does not terminate. Reproduced deterministically in both forms, with controls that pass:

Call Result
extract(cap, nofile=True, no_eof=True) hangs; faulthandler stack in pcapkit/protocols/protocol.py __init__
extract(cap, nofile=True, auto=False, store=False, no_eof=True) then iterate hangs; stack in logging/__init__.py makeRecord
same, without no_eof (control) returns, frames=6
same, without no_eof, iterating (control) iteration ends, seen=6

ExtractionWarning: EOF reached is emitted in all four cases, including the hanging ones, so EOF is detected — what no_eof suppresses is the EOFError that stops the loop, and nothing else stops it. No library code was touched here.

@JarryShaw

Copy link
Copy Markdown
Owner Author

✅ GOOD TO MERGE — head 2af230041ca37fd740349b348c7e3c0c9ba7d568. Built the distribution and ran twine check myself: both artifacts PASSED, wheel METADATA carries Description-Content-Type: text/markdown with a genuinely Markdown body, and the sdist ships README.md at root with no stray root README.rst. All seven "already duplicated" blocks confirmed present in docs/source/index.rst, both "moved" blocks confirmed landed, and the examples/benchmark/Dockerfile fix reproduced in both directions with a real docker build. One reviewer-side correction worth recording: an earlier pass of mine flagged CONTRIBUTING.md:9 as a live dangling README.rst reference contradicting the PR body — that was wrong, and the PR body is right. See appendix.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review appendix — PR #619

Reviewer: Sonnet; PR authored on Opus 5. Reviewed at head 2af230041ca37fd740349b348c7e3c0c9ba7d568 in an isolated worktree.

Acceptance test — built it and ran twine check rather than trusting the reported PASS

setup.py:32-33 confirmed reading README.md; setup.py:181 confirmed long_description_content_type="text/markdown"; MANIFEST.in:18 confirmed include README.md.

PYTHONSAFEPATH=1 python -m build --sdist --wheel --outdir /tmp/pr619-verify .
→ Successfully built pypcapkit-1.5.0b4.tar.gz and pypcapkit-1.5.0b4-py3-none-any.whl

python -m twine check ./*
→ Checking pypcapkit-1.5.0b4-py3-none-any.whl: PASSED
→ Checking pypcapkit-1.5.0b4.tar.gz: PASSED

Went past the PASS and read the artifacts: wheel METADATA carries Description-Content-Type: text/markdown, its body's first line is # PyPCAPKit -- Comprehensive Network Packet Analysis Library (real Markdown, not leftover RST), and the sdist contains pypcapkit-1.5.0b4/README.md at root with no root README.rst. MANIFEST.in's global-include *.rst would not have matched a .md file, so include README.md is genuinely load-bearing — confirmed by the tarball contents, not by the comment asserting it.

Nothing lost

Old README.rst 424 lines → new README.md 103. All seven blocks claimed already-duplicated (Module Structure, Engine Comparison, engine-support-by-Python-version table, Test Environment, Test Results + footnotes, Installation Notes, engine prerequisites) confirmed present in docs/source/index.rst in equal or greater detail. The two blocks that existed nowhere else were genuinely moved, not dropped: docs/source/testing.rst is new (53 lines, ADDED in --stat) and registered in the index.rst toctree, and the pipenv/make setup block is verbatim in the docs' Installation section.

The Dockerfile fix — reproduced both directions

examples/benchmark/Dockerfile:156 went from COPY pyproject.toml setup.py MANIFEST.in README.rst ./ to ... README.md ./, and run.sh confirms the build context is the repo root. Against that same real context and a pinned base image, an isolated two-COPY build shows the fixed line succeeding (README.md, 4536 bytes) and the pre-fix line failing with ERROR: ... "/README.rst": not found. The full multi-stage build (compiling pypcap, apt-get libpcap-dev) was not attempted and would add nothing over an isolated reproduction on the identical context.

Dangling references — the full picture, and a correction to my own earlier finding

Only three README.rst references survive anywhere in this PR's tree, and all three are correct:

  • examples/benchmark/benchmark.py:25 — deliberately left alone, and verified correct: the docstring sits inside examples/benchmark/, so :file:README.rst`` resolves to examples/benchmark/README.rst, which exists and carries "Departures from the legacy methodology" at its line 247.
  • tests/project/test_changelog_md.py:432 — a string-literal fixture, read in context: it asserts that prose about a :mod: role (quoting a historical PyPI rejection) is not mistaken for an escaped role. Not a file reference.
  • CONTRIBUTING.md:9"- Read the README.rst for build instructions." present in this branch's tree.

On that third one I initially called NEEDS CHANGES, on the grounds that the PR body claims it was "handled separately by dropping the extension" while git diff <base>..HEAD -- CONTRIBUTING.md is empty. That conclusion was wrong and I am withdrawing it. The claim is about a sibling PR, and that sibling has since merged: #613 ("docs: repair the Code of Conduct's rendering and refresh CONTRIBUTING", merge 4ecac90a1) rewrote the file, and on current main the line reads "Read the README for installation and build instructions, and its Testing section for the test suite" — extension dropped, exactly as claimed. My error was checking the reference in this branch's own tree, which forks from a2be2cc1a and therefore predates #613. Since this PR must rebase before merge anyway, the merged result carries #613's version and no dangling reference remains.

The remaining ~20 root-README.rst references that exist on main (Makefile:90, setup.py:32, MANIFEST.in:14, and a dozen sites across examples/benchmark/report.py, run.sh, test_harness.py, README.rst) are all corrected by this PR — its --stat touches every one of those files, which substantiates the "24 prose sites" claim.

Not independently verified

The PR's "167 tests pass" and "scoped Sphinx build succeeded" claims were not reproduced. The shared venv's pcapkit is an editable install pinned to the main checkout (__editable__.pypcapkit-1.4.1.post2.pth), so its import hook resolves import pcapkit to the main tree even under PYTHONSAFEPATH=1 with cwd in the worktree — an installed finder is not defeated by PYTHONSAFEPATH. Running Sphinx (whose conf.py imports pcapkit) or the suite from that venv would silently exercise the wrong tree, so it was flagged rather than faked. The one self-contained file that matters for the acceptance test, tests/project/test_setup.py (loads setup.py by path, never imports pcapkit), passed 4/4 with rootdir confirmed as the worktree.

Merge state

Live GraphQL reads mergeable: CONFLICTING / mergeStateStatus: DIRTY against current main (c260f705b). git merge-tree localises the conflict to a CHANGELOG.md insertion-point collision with the since-merged #614 — mechanical, not a content defect. Also note the review brief's stated base af1f771b9 is not an ancestor of this branch; the true merge-base is a2be2cc1a.

Disagreement log

One disagreement, and it is with my own first pass rather than with the PR: the CONTRIBUTING.md:9 NEEDS CHANGES finding is withdrawn, because #613 has merged and dropped that extension exactly as the PR body said it would. Everything else — the build/twine check acceptance test, all nine duplication/move claims, the Dockerfile fix reproduced in both directions, and the two intentional surviving references — held up under independent reproduction.

Comment thread README.md Outdated
with it. When that depth is not what you need, the same interface will also drive
six third-party extraction engines instead.

The whole project supports **Python 3.6** or later; CI covers 3.10 to 3.14, and

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop the part about CI and 3.15.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. The clause after the semicolon is gone; the paragraph is now a single line:

The whole project supports Python 3.6 or later.

Grepped the rest of README.md for any other CI or 3.15 mention and there is none — this was the only site. Left docs/source/index.rst alone, where the CI matrix belongs, and pyproject.toml:47's comment on the same subject predates this PR and is untouched by it.

Also rebased onto current main (dfc23b8b8, which picked up #621). The only conflicts were the two changelog files, both at the usual shared anchor: docs/source/changelog/1.5.0.rst resolved by hand keeping every bullet from both sides with this PR's last, and CHANGELOG.md discarded and regenerated with util/changelog_md.py--check exits 0. Re-verified the packaging: python -m build clean, twine check --strict PASSED on both sdist and wheel, and pypcapkit-1.5.0b4/README.md ships at the sdist root.

@JarryShaw

Copy link
Copy Markdown
Owner Author

✅ GOOD TO MERGE — head 2af230041ca37fd740349b348c7e3c0c9ba7d568, subject to a changelog-only rebase. Independently rebuilt the sdist and wheel from this exact tree and reproduced twine check --strict PASSED/PASSED, and confirmed README.md actually ships inside the sdist — the load-bearing pair, since setup.py reads the README unguarded and an sdist without it could not be installed. See appendix.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review appendix — PR #619

Reviewer: Sonnet; PR authored on Opus 5. Reviewed at head 2af230041ca37fd740349b348c7e3c0c9ba7d568 in an isolated worktree.

The acceptance test, reproduced rather than taken on report

Built from this worktree (not the shared checkout): python -m build --sdist --wheel, exit 0, then

Checking pypcapkit-1.5.0b4-py3-none-any.whl: PASSED
Checking pypcapkit-1.5.0b4.tar.gz: PASSED

twine check --strict, exit 0 — reproduced with and without --strict. This is the check that matters, because setup.py:181 had to move from text/x-rst to text/markdown or the rendered long description would be rejected. The wheel's METADATA confirms Description-Content-Type: text/markdown, Metadata-Version: 2.4.

The failure mode that would actually have bitten: README.md must ship in the sdist

setup.py's get_long_description() opens the README unguarded — no try, no existence check — so a source distribution that omits it fails at install time, not at build time. MANIFEST.in:3 is global-include *.rst, which stopped matching the README the moment it became Markdown, leaving MANIFEST.in:18's include README.md as the only thing putting it in the sdist. Verified directly:

tar tzf pypcapkit-1.5.0b4.tar.gz | grep -i README
  pypcapkit-1.5.0b4/README.md
  pypcapkit-1.5.0b4/examples/benchmark/README.rst
  pypcapkit-1.5.0b4/examples/legacy_smoke/README.rst

README.md is present. The two .rst survivors are the benchmark and legacy-smoke suites' own READMEs, correctly unrelated to the root one. The in-file comment at MANIFEST.in:14-17 calling that line "load-bearing, not belt-and-braces" is accurate.

setup.py

git diff af1f771b9...2af230041 -- setup.py is exactly the two expected changes and nothing else: get_long_description() opens README.md, and long_description_content_type is text/markdown.

Line counts and Markdown sanity

git show af1f771b9:README.rst | wc -l424. wc -l README.md103. Exactly as claimed. No leftover RST syntax in the new file; both repo-relative links resolve.

Content migration — seven duplicated, two moved

The seven blocks dropped as already-duplicated (Module Structure, Engine Comparison, Engine support by Python version, Test Environment, Test Results, Installation Notes, Engine prerequisites) were each located in docs/source/index.rst with matching substance, including the Test Results benchmark figures byte-for-byte. The two that existed nowhere else did move rather than vanish: docs/source/testing.rst is a real new file carrying the old Testing section in full (make test, the untracked-fixtures caveat, make samples/make test-all, the option round-trip generator paragraph), registered in the index toctree; the pipenv/make setup block is visible in this PR's own +23/-0 diff to docs/source/index.rst's Installation section.

The Dockerfile break it found and fixed

examples/benchmark/Dockerfile did a literal COPY pyproject.toml setup.py MANIFEST.in README.rst ./ — no glob, and there is no .dockerignore anywhere in the repo. Reproduced with real docker build runs isolating just that COPY against trees pulled from each commit: the old line against a tree carrying only README.md fails with failed to compute cache key: ... "/README.rst": not found; the new line against the same tree succeeds; the old line against the old tree succeeds as a control. A genuine break, correctly caught and fixed inside the change that caused it.

Correction: README.rst references surviving at this head

A full grep at this head finds four non-narrative references, not one:

  • examples/benchmark/benchmark.py:25 — correctly left alone; it names the benchmark suite's own README in the same directory, confirmed by reading its "Departures from the legacy methodology" context.
  • tests/project/test_changelog_md.py:432 — a string fixture describing the historical 1.5.0b1 PyPI rejection. Not a live path reference.
  • CHANGELOG.md:67, docs/source/changelog/1.5.0.rst:818-858 — this change's own narrative describing the rename. Correct as prose.
  • CONTRIBUTING.md:9 — "Read the README.rst for build instructions." A genuinely live, now-wrong reference at this head. This PR does not touch CONTRIBUTING.md. It is already fixed on main by the separately-merged docs: repair the Code of Conduct's rendering and refresh CONTRIBUTING #613 (4ecac90a1), which rewrites that line to "Read the README for installation and build instructions" — verified by reading both sides. So it resolves on rebase and is not a defect in this PR, but it does mean this change was only ever complete in combination with docs: repair the Code of Conduct's rendering and refresh CONTRIBUTING #613.

For the record, this PR does correctly clear the references that still exist on main and would otherwise have been left dangling — Makefile:90, examples/benchmark/report.py:112,288,882 and the benchmark suite's own README.rst all still name the root README.rst on main and are absent at this head.

Branch state: needs a rebase

The branch's merge-base with main is a2be2cc1a, now four commits behind the tip. A forced GraphQL read returns mergeStateStatus: DIRTY, mergeable: CONFLICTING. git merge-tree --write-tree origin/main 2af230041 (read-only) confines the conflict to CHANGELOG.md and docs/source/changelog/1.5.0.rst — two append-only logs where this PR's bullet collides with siblings landed at the same anchor. Routine last-mover churn, not a substantive conflict, but it must be rebased before it can merge.

One diff-hygiene note for anyone re-checking this: the two-dot af1f771b9..2af230041 spuriously lists SECURITY.md (15 files) purely from that base drift — #611 touched it on main after this branch point. The three-dot af1f771b9...2af230041 and gh pr diff 619 --name-only agree at 14 files with no SECURITY.md. This PR does not touch that file.

Not independently checked

  • The "16 links resolved" claim in full — the two repo-relative links were confirmed to exist; the docs-site anchors and external URLs were not crawled.
  • A full Sphinx HTML build of the docs tree — attempted and abandoned at ~32% as impractical for this review's budget. The structural facts (toctree entry, testing.rst with real content) are confirmed without it.
  • The PR body's "no license section added" and the separately-filed no_eof=True hang — out of scope for this diff.
  • pcapkit/__init__.py's docstring, cited as another place Module Structure already lived.

Disagreement log

One correction, against the review brief rather than against the PR: the brief asserted that examples/benchmark/benchmark.py:25 was the only surviving README.rst reference. It is not — CONTRIBUTING.md:9 is a live and newly-wrong reference at this head, and the PR does not fix it. The PR's own body is the more accurate account, since it names that site explicitly and says it is handled separately; that claim checks out against #613 on main. No disagreement with anything the PR itself asserts — every testable claim in it reproduced exactly, including the error string from the Dockerfile repro and the 167 passed / 37 subtests count.

Requested by the project owner: drop the technical detail from the README, and
change it back to Markdown.

* Trimmed `README.rst` (424 lines) to `README.md` (103). What stays is what a
  reader arriving from PyPI or a search result needs: what the library is, why it
  exists rather than Scapy or DPKT, how to install it, a worked example, and
  where the documentation is. Added the worked example, which the README did not
  have before; its output is from the committed `examples/captures/in.pcap` and
  every value in it was measured against this tree.
* Removed seven blocks that `docs/source/index.rst` already carried in a fuller
  form, and linked them instead: Module Structure, Engine Comparison, Engine
  support by Python version, Test Environment, Test Results, Installation Notes,
  and Engine prerequisites.
* Moved the two blocks that existed nowhere else rather than dropping them. The
  Testing section is now `docs/source/testing.rst`, registered in the index
  toctree; the pipenv and `make setup` local development block joined the
  Installation section of `docs/source/index.rst`.
* `setup.py` reads `README.md` and declares `text/markdown`. The
  `include README.md` in `MANIFEST.in` is now load-bearing, since
  `global-include *.rst` no longer matches the README and `setup.py` reads it
  unguarded.
* Updated the references to the renamed file, since a change that renames a file
  owns the references to it. `examples/benchmark/Dockerfile` copies `README.md`;
  a literal `COPY` of the old name failed the layer outright, taking `make bench`,
  `make bench-quick` and `run.sh` with it. The benchmark harness prose that named
  the root README as where its generated tables land now names
  `docs/source/index.rst`, which is where those tables went: the `Makefile`
  comment, `report.py`, `test_harness.py`, `run.sh` and the suite's own README.
  One of those had a reason that inverted rather than just a stale path -- the
  emitted markup stays parseable by plain docutils, but that is now a
  conservative choice rather than a hard requirement, because the page it lands
  on is rendered by Sphinx.

This crosses the convention that documentation here is reStructuredText. It is
deliberate and scoped to the README, which is the one documentation file rendered
by GitHub and PyPI rather than by Sphinx. No other `.rst` file was converted.

Left alone on purpose: `examples/benchmark/benchmark.py:25` still says
`README.rst`, correctly, because it means the benchmark suite's own README in the
same directory; and `tests/project/test_changelog_md.py:432` is a string fixture
describing the historical `1.5.0b1` release failure.

Verified: `twine check --strict` passes on a built sdist and wheel, the README
ships in the sdist, and the metadata declares `text/markdown`. A `FROM scratch`
build against the same context `run.sh` uses resolves the new `COPY` line and
fails the old one with `"/README.rst": not found`. Tests pass --
`tests/project/test_setup.py` (4), `tests/project/test_changelog_md.py` (47) and
`examples/benchmark/test_harness.py` (116) -- and
`python util/changelog_md.py --check` exits 0.
@JarryShaw
JarryShaw force-pushed the docs/readme-landing-page-markdown branch from 2af2300 to eae2f57 Compare September 22, 2026 03:34
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review reconciliation — PR #619, the CONTRIBUTING.md:9 question

Reviewer: Sonnet; PR authored on Opus 5. A second, independent pass over this PR reached the opposite conclusion from the retraction recorded above ("an earlier pass of mine flagged CONTRIBUTING.md:9 ... that was wrong"). Both passes were looking at real evidence; they were looking at different trees. Recording the resolution so the thread does not leave a misleading correction standing.

At this PR's head, the reference genuinely is dangling

$ git show 2af230041:CONTRIBUTING.md | sed -n '9p'
- Read the README.rst for build instructions.

$ git ls-tree 2af230041 README.rst README.md
100644 blob 024f3221f926809b635a8f52a144521efe6cff74	README.md

No root README.rst in that tree — only README.md. So at 2af230041ca37fd740349b348c7e3c0c9ba7d568, a contributor following CONTRIBUTING.md:9 literally is pointed at a file that does not exist. The earlier flag was not wrong about the head it was raised against.

On current main it is already fixed — by #613, not by this PR

$ git show origin/main:CONTRIBUTING.md | grep -n README
5:...The README is the authority on installing and building; this file covers what
11:- Read the README for installation and build instructions, and its *Testing* section for the test

#613 (4ecac90a1, "repair the Code of Conduct's rendering and refresh CONTRIBUTING") rewrote CONTRIBUTING.md wholesale — 152 lines — and in doing so dropped the extension entirely. And #613 is not an ancestor of this PR's head:

$ git merge-base --is-ancestor 4ecac90a1 2af230041   # → false

which is the whole explanation: this PR was cut before #613 landed, so it still carries the pre-#613 CONTRIBUTING.md.

Net: not this PR's problem, and not reintroduced by it

gh pr diff 619 --name-only does not include CONTRIBUTING.md — this PR never touches the file. Merging it onto current main therefore takes main's already-fixed version; the stale line cannot come back. No action needed on this PR, and the **✅ GOOD TO MERGE** verdict above is unaffected.

The retraction's conclusion (nothing for #619 to fix) was right; its stated reason (the flag "was wrong") was not. The flag was correct about the reviewed head and was overtaken by a sibling PR fixing it independently — worth distinguishing, because anyone who later checks out 2af230041 to reproduce this review will find the dangling reference exactly where the first pass said it was.

@JarryShaw
JarryShaw merged commit 2221c2d into main Sep 22, 2026
12 of 24 checks passed
@JarryShaw
JarryShaw deleted the docs/readme-landing-page-markdown branch September 22, 2026 03:38
JarryShaw added a commit that referenced this pull request Sep 22, 2026
Requested by the owner; no issue is open for it.

* `MANIFEST.in` had `include` lines for `README.md`, `LICENSE` and
  `CHANGELOG.md` but none for `CITATION.cff`, and its two `global-include`
  patterns are `*.rst` and `*.py`, neither of which matches a `.cff`. The file
  was therefore in the repository and in no source distribution.
* The gap is invisible from the web UI, since GitHub renders the "Cite this
  repository" button from the repository. Citation managers, Zenodo and
  dependency inventories read the published artifact, which is the surface that
  was missing it.
* It matters now because #625 has just taught `util/bump_version.py` to keep
  that file's `version` and `date-released` in step with the bump, so a release
  exercising that path would publish an sdist omitting the artefact under test.
  #615 flagged the omission when it added the file; #619 did not address it.
* The new line carries a comment, as its neighbours do, saying why it is
  load-bearing rather than belt-and-braces.
* Changelog bullet added; `CHANGELOG.md` regenerated with
  `util/changelog_md.py`, and `--check` exits 0.

Measured both ways with `python -m build --sdist`: `tar tzf | grep -i citation`
found nothing before and `pypcapkit-1.5.0b4/CITATION.cff` after; the two archive
listings differ by that one entry and nothing else, 860 against 861; the shipped
copy is byte-identical to the repository's; and `twine check --strict` reports
PASSED on both. No tests were run -- one line of packaging configuration,
nothing under `pcapkit/`.
JarryShaw added a commit that referenced this pull request Sep 22, 2026
Requested by the owner; no issue is open for it.

* `MANIFEST.in` had `include` lines for `README.md`, `LICENSE` and
  `CHANGELOG.md` but none for `CITATION.cff`, and its two `global-include`
  patterns are `*.rst` and `*.py`, neither of which matches a `.cff`. The file
  was therefore in the repository and in no source distribution.
* Nor was there a default to fall back on. Removing all three `include` lines
  and rebuilding shows `README.md` and `LICENSE` shipping regardless --
  setuptools adds the latter from `license_files`, logging `adding license file
  'LICENSE'` and recording `License-File: LICENSE` in `PKG-INFO` -- while
  `CHANGELOG.md` disappears. Of the three only `CHANGELOG.md` is load-bearing,
  and a citation file, which nothing outside `MANIFEST.in` knows about, is in
  the same position.
* The gap is invisible from the web UI, since GitHub renders the "Cite this
  repository" button from the repository. Citation managers, Zenodo and
  dependency inventories read the published artifact, which is the surface that
  was missing it.
* It matters now because #625 has just taught `util/bump_version.py` to keep
  that file's `version` and `date-released` in step with the bump, so a release
  exercising that path would publish an sdist omitting the artefact under test.
  #615 flagged the omission when it added the file; #619 did not address it.
* Changelog bullet added; `CHANGELOG.md` regenerated with
  `util/changelog_md.py`, and `--check` exits 0.

Measured both ways with `python -m build --sdist`: `tar tzf | grep -i citation`
found nothing before and `pypcapkit-1.5.0b4/CITATION.cff` after; the two archive
listings differ by that one entry and nothing else, 860 against 861; the shipped
copy is byte-identical to the repository's; and `twine check --strict` reports
PASSED on both. No tests were run -- one line of packaging configuration,
nothing under `pcapkit/`.
JarryShaw added a commit that referenced this pull request Sep 22, 2026
Requested by the owner; no issue is open for it.

* `MANIFEST.in` had `include` lines for `README.md`, `LICENSE` and
  `CHANGELOG.md` but none for `CITATION.cff`, and its two `global-include`
  patterns are `*.rst` and `*.py`, neither of which matches a `.cff`. The file
  was therefore in the repository and in no source distribution.
* Nor was there a default to fall back on. Removing all three `include` lines
  and rebuilding shows `README.md` and `LICENSE` shipping regardless --
  setuptools adds the latter from `license_files`, logging `adding license file
  'LICENSE'` and recording `License-File: LICENSE` in `PKG-INFO` -- while
  `CHANGELOG.md` disappears. Of the three only `CHANGELOG.md` is load-bearing,
  and a citation file, which no packaging default covers at all, is in the same
  position.
* The gap is invisible from the web UI, since GitHub renders the "Cite this
  repository" button from the repository. Citation managers, Zenodo and
  dependency inventories read the published artifact, which is the surface that
  was missing it.
* It matters now because #625 has just taught `util/bump_version.py` to keep
  that file's `version` and `date-released` in step with the bump, so a release
  exercising that path would publish an sdist omitting the artefact under test.
  #615 flagged the omission when it added the file; #619 did not address it.
* It also lets `RepositoryCitationTests` in `tests/project/test_bump_version.py`
  -- the gate #625 added for the hand-authored bumps that never run the script --
  execute against an unpacked sdist, where today it skips itself with
  "CITATION.cff is not shipped in the source distribution".
* Changelog bullet added; `CHANGELOG.md` regenerated with
  `util/changelog_md.py`, and `--check` exits 0.

Rebased onto `cfb81d3f6` after #630 merged; the changelog conflict was resolved
keeping both bullets and regenerating `CHANGELOG.md` rather than merging it.

Measured both ways with `python -m build --sdist` against that base: `tar tzf |
grep -i citation` found nothing before and `pypcapkit-1.5.0b4/CITATION.cff`
after; the listings differ by that one entry and nothing else, 860 against 861;
the shipped copy is byte-identical to the repository's; and `twine check
--strict` reports PASSED on both. No tests were run -- one line of packaging
configuration, nothing under `pcapkit/`.
@JarryShaw JarryShaw added the docs Pull requests that change documentation only (docs: subject prefix) label Sep 22, 2026
JarryShaw added a commit that referenced this pull request Sep 22, 2026
…two false packaging claims (#642)

Three names appeared in string annotations that their module never imported, and
they were mypy's complete set of ``name-defined`` findings for the package:

* ``pcapkit/utilities/logging.py:350`` used ``Any``; added to the
  ``TYPE_CHECKING`` block beside ``IO``, ``Optional`` and ``Union``.
* ``pcapkit/protocols/schema/internet/ipv6_route.py:136`` used ``Protocol`` and
  ``:271`` used ``Optional``; added, ``Protocol`` as
  ``ProtocolBase as Protocol``. Twenty-one sibling schema modules already spell
  it that way, in the same ``payload:`` stub; this makes twenty-two.

mypy 2.3.1 over ``pcapkit``: 3 ``name-defined`` errors before, 0 after; 115 total
errors before, 112 after, so nothing else moved.

``MANIFEST.in:14-17`` asserted that ``include README.md`` was "the only thing
that puts it in an sdist" and that an sdist without it "cannot be installed at
all". Both halves are false, and the file had contradicted itself since #631
wrote the correct mechanism seven lines below without correcting this. Deleting
the lines and rebuilding gives a byte-identical sdist listing -- empty diff --
that installs with exit 0: ``setuptools/command/sdist.py:59-60`` ships the README
unconditionally and ``setuptools/dist.py:460``'s default ``license_files`` glob
ships ``LICENSE``. Of the original three ``include`` lines only ``CHANGELOG.md``
is load-bearing. The comment now says that, and the same correction is applied to
the #619 entry in ``docs/source/changelog/1.5.0.rst``, which carried the
identical claim.

Three changelog bullets gained the citation they were missing: ``(#619)`` for the
README entry, ``(#570)`` for the ``code=`` keyword and ``(#577)`` for the
``StreamEOFError`` gaps. Two of the five remain uncited because no number could
be recovered for them.

``CHANGELOG.md`` regenerated with ``util/changelog_md.py``; ``--check`` exits 0.

New ``tests/project/test_annotation_names.py`` resolves every string annotation
in the package -- following a nested forward reference such as
``'list["Nested"]'``, while treating ``Literal`` members and ``Annotated``
metadata as the values they are -- against the names its own module binds. It
reports the same three findings as mypy on the unfixed tree and none after.
128 -> 132 passed over ``tests/project`` and ``tests/utilities/test_logging.py``,
both exit 0, subtests unchanged at 487.
JarryShaw added a commit that referenced this pull request Sep 22, 2026
…two false packaging claims (#642)

Three names appeared in string annotations that their module never imported, and
they were mypy's complete set of ``name-defined`` findings for the package:

* ``pcapkit/utilities/logging.py:350`` used ``Any``; added to the
  ``TYPE_CHECKING`` block beside ``IO``, ``Optional`` and ``Union``.
* ``pcapkit/protocols/schema/internet/ipv6_route.py:136`` used ``Protocol`` and
  ``:271`` used ``Optional``; added, ``Protocol`` as
  ``ProtocolBase as Protocol``. Twenty-one sibling schema modules already spell
  it that way, in the same ``payload:`` stub; this makes twenty-two.

mypy 2.3.1 over ``pcapkit``: 3 ``name-defined`` errors before, 0 after; 115 total
errors before, 112 after, so nothing else moved.

``MANIFEST.in:14-17`` asserted that ``include README.md`` was "the only thing
that puts it in an sdist" and that an sdist without it "cannot be installed at
all". Both halves are false, and the file had contradicted itself since #631
wrote the correct mechanism seven lines below without correcting this. Deleting
the lines and rebuilding gives a byte-identical sdist listing -- empty diff --
that installs with exit 0: ``setuptools/command/sdist.py:59-60`` ships the README
unconditionally and ``setuptools/dist.py:460``'s default ``license_files`` glob
ships ``LICENSE``. Of the original three ``include`` lines only ``CHANGELOG.md``
is load-bearing. The comment now says that, and the same correction is applied to
the #619 entry in ``docs/source/changelog/1.5.0.rst``, which carried the
identical claim.

Three changelog bullets gained the citation they were missing: ``(#619)`` for the
README entry, ``(#570)`` for the ``code=`` keyword and ``(#577)`` for the
``StreamEOFError`` gaps. Two of the five remain uncited because no number could
be recovered for them.

``CHANGELOG.md`` regenerated with ``util/changelog_md.py``; ``--check`` exits 0.

New ``tests/project/test_annotation_names.py`` resolves every string annotation
in the package -- following a nested forward reference such as
``'list["Nested"]'``, while treating ``Literal`` members and ``Annotated``
metadata as the values they are -- against the names its own module binds. It
reports the same three findings as mypy on the unfixed tree and none after.
127 -> 132 passed over ``tests/project`` and ``tests/utilities/test_logging.py``,
both exit 0, subtests unchanged at 487.
JarryShaw added a commit that referenced this pull request Sep 22, 2026
…a false packaging claim (#642)

Three names appeared in string annotations that their module never imported, and
they were mypy's complete set of ``name-defined`` findings for the package:

* ``pcapkit/utilities/logging.py:350`` used ``Any``; added to the
  ``TYPE_CHECKING`` block beside ``IO``, ``Optional`` and ``Union``.
* ``pcapkit/protocols/schema/internet/ipv6_route.py:136`` used ``Protocol`` and
  ``:271`` used ``Optional``; added, ``Protocol`` as
  ``ProtocolBase as Protocol``. Twenty-one sibling schema modules already spell
  it that way, in the same ``payload:`` stub; this makes twenty-two.

mypy 2.3.1 over ``pcapkit``: 3 ``name-defined`` errors before, 0 after; 115 total
errors before, 112 after, so nothing else moved.

``MANIFEST.in:14-17`` asserted that ``include README.md`` was "the only thing
that puts it in an sdist" and that an sdist without it "cannot be installed at
all". Both halves are false, and the file had contradicted itself since #631
wrote the correct mechanism seven lines below without correcting this. Deleting
the lines and rebuilding gives a byte-identical sdist listing -- empty diff --
that installs with exit 0: ``setuptools/command/sdist.py:59-60`` ships the README
unconditionally and ``setuptools/dist.py:460``'s default ``license_files`` glob
ships ``LICENSE``. Of the original three ``include`` lines only ``CHANGELOG.md``
is load-bearing. The comment now says that.

New ``tests/project/test_annotation_names.py`` resolves every string annotation
in the package -- following a nested forward reference such as
``'list["Nested"]'``, while treating ``Literal`` members and ``Annotated``
metadata as the values they are -- against the names its own module binds. It
reports the same three findings as mypy on the unfixed tree and none after.

That module named ``ast.TypeAlias`` and ``ast.TypeVar`` directly, and both are
PEP 695 nodes added in Python 3.12, so *every* test in it raised
``AttributeError`` on the 3.10 and 3.11 matrix jobs -- ``bound_names`` walks every
node of every file, so the attribute is reached whatever a test does. Both are now
resolved once at module scope through ``getattr(ast, ..., ())``, leaving the
``isinstance`` branches otherwise untouched: ``isinstance(x, ())`` is always
False, so the branches stay live on 3.12+ and are simply unreachable below it.
Chosen over a ``sys.version_info`` comparison because it writes no version number
down at all -- a comparison states 3.12 next to the attribute it guards, and the
two can then drift -- and over a per-node ``getattr`` because a module-level
constant lifts the lookup out of a loop that runs on every node of every file.

``ast.TypeVar`` is the branch that earns its keep: it carries its name as a bare
``str`` and emits no ``ast.Name`` node, so forcing ``_TYPE_VAR`` to ``()`` on
3.14.7 turns ``T`` and ``U`` into false findings. ``ast.TypeAlias`` is defensive
by comparison -- its name *is* an ``ast.Name`` in ``Store`` context, which the
preceding branch already catches -- and is left as it stands rather than removed.
A new ``test_a_pep695_type_parameter_is_in_scope`` pins both the guards and the
behaviour, skipped below 3.12 because its fixture source cannot parse there.

Measured on real interpreters rather than simulated. 3.10.21 and 3.11.15:
5 failed, exit 1 -> 5 passed, 1 skipped, exit 0. 3.14.7: all 6 pass, exit 0.
133 passed over ``tests/project`` and ``tests/utilities/test_logging.py``, exit 0,
subtests unchanged at 487.

No changelog entry on this branch. Per the rule that no code branch touches
``CHANGELOG.md`` or anything under ``docs/source/changelog/``, this change's entry
-- and the wording correction the ``MANIFEST.in`` claim implies for the #619
entry, plus the missing ``(#570)`` and ``(#577)`` citations -- go to the shared
changelog pull request #657 instead.
JarryShaw added a commit that referenced this pull request Sep 22, 2026
The changelog changes #666 was carrying on its own branch, moved here so that #666
touches only `MANIFEST.in`, `pcapkit/protocols/schema/internet/ipv6_route.py`,
`pcapkit/utilities/logging.py` and `tests/project/test_annotation_names.py`. #666
was the last open branch still editing `CHANGELOG.md` itself; it no longer does.

Four pieces, not one, because #666 had amended existing entries as well as needing
a new one.

A new **Fixed** bullet for #642: three names used in string annotations that their
own module never imported -- `Any` in `pcapkit/utilities/logging.py`, and
`Protocol` and `Optional` in `pcapkit/protocols/schema/internet/ipv6_route.py`.
The bullet names the `typing.cast` case specifically, because that is the one no
running test can catch: `cast` never evaluates its first argument. It states
plainly that nothing resolves at runtime that did not before, since `TYPE_CHECKING`
is `False` when the interpreter runs, so that the entry is not read as a runtime
fix. mypy 2.3.1's before/after is quoted as the measurement -- three `name-defined`
errors to none, 115 total to 112 -- and the new
`tests/project/test_annotation_names.py` is described as what pins the invariant
without a type checker installed.

Three missing citations recovered: `(#570)` on the L2TPv3 worked-example line,
`(#577)` on the `register_extractor_engine` keyword line, and `(#619)` on the
README rename entry.

And the #619 entry's packaging claim corrected in place. It asserted that
`include README.md` in `MANIFEST.in` was "the only thing that puts the README in a
source distribution" and that an sdist without it "cannot be installed". Both
halves are false: setuptools' own `sdist` command ships the README before
`MANIFEST.in` is read at all, so dropping the line leaves the listing
byte-identical at 861 entries, and `setup.py` reads the file from wherever it is
executing, which for a `pip` install of an sdist is the unpacked sdist. #666
corrects the same claim in the `MANIFEST.in` comment, so the two stay in step.

No `:pep:` role, though the new bullet discusses PEP 695: `util/changelog_md.py`
converts only double-backtick literals and the `:rfc:` role, and raises
`ResidualMarkupError` on anything else, exactly as the #661 entry hit with `:obj:`.
Plain prose instead.

50 lines added to the two files, 11 reflowed. `CHANGELOG.md` regenerated with
`util/changelog_md.py`, not edited; `--check` exits 0 and `tests/project/` is green
at 96 passed, 469 subtests, exit 0 -- the same counts the previous commit on this
branch reported, so nothing else moved.

Committed from a detached HEAD and pushed to the branch ref, because
`docs/changelog-1.5.0` is checked out in another agent's worktree at a stale
f846523 and could not be taken here. Written against b2ec64b and rebased onto
d47ada0, the branch having taken #641's entry and the #652/#650 boundary
correction in the meantime. Both files conflicted, both at the append point rather
than in substance -- #641's bullet and this one land in the same place at the end
of **Fixed** -- so the resolution keeps both, #641's first. `CHANGELOG.md` was not
hand-resolved: it is generated, so it was regenerated from the resolved entry file
and `--check` re-run, which is the only resolution that cannot drift.

Refs #642
JarryShaw added a commit that referenced this pull request Sep 22, 2026
…a false packaging claim (#642) (#666)

Three names appeared in string annotations that their module never imported, and
they were mypy's complete set of ``name-defined`` findings for the package:

* ``pcapkit/utilities/logging.py:350`` used ``Any``; added to the
  ``TYPE_CHECKING`` block beside ``IO``, ``Optional`` and ``Union``.
* ``pcapkit/protocols/schema/internet/ipv6_route.py:136`` used ``Protocol`` and
  ``:271`` used ``Optional``; added, ``Protocol`` as
  ``ProtocolBase as Protocol``. Twenty-one sibling schema modules already spell
  it that way, in the same ``payload:`` stub; this makes twenty-two.

mypy 2.3.1 over ``pcapkit``: 3 ``name-defined`` errors before, 0 after; 115 total
errors before, 112 after, so nothing else moved.

``MANIFEST.in:14-17`` asserted that ``include README.md`` was "the only thing
that puts it in an sdist" and that an sdist without it "cannot be installed at
all". Both halves are false, and the file had contradicted itself since #631
wrote the correct mechanism seven lines below without correcting this. Deleting
the lines and rebuilding gives a byte-identical sdist listing -- empty diff --
that installs with exit 0: ``setuptools/command/sdist.py:59-60`` ships the README
unconditionally and ``setuptools/dist.py:460``'s default ``license_files`` glob
ships ``LICENSE``. Of the original three ``include`` lines only ``CHANGELOG.md``
is load-bearing. The comment now says that.

New ``tests/project/test_annotation_names.py`` resolves every string annotation
in the package -- following a nested forward reference such as
``'list["Nested"]'``, while treating ``Literal`` members and ``Annotated``
metadata as the values they are -- against the names its own module binds. It
reports the same three findings as mypy on the unfixed tree and none after.

That module named ``ast.TypeAlias`` and ``ast.TypeVar`` directly, and both are
PEP 695 nodes added in Python 3.12, so *every* test in it raised
``AttributeError`` on the 3.10 and 3.11 matrix jobs -- ``bound_names`` walks every
node of every file, so the attribute is reached whatever a test does. Both are now
resolved once at module scope through ``getattr(ast, ..., ())``, leaving the
``isinstance`` branches otherwise untouched: ``isinstance(x, ())`` is always
False, so the branches stay live on 3.12+ and are simply unreachable below it.
Chosen over a ``sys.version_info`` comparison because it writes no version number
down at all -- a comparison states 3.12 next to the attribute it guards, and the
two can then drift -- and over a per-node ``getattr`` because a module-level
constant lifts the lookup out of a loop that runs on every node of every file.

``ast.TypeVar`` is the branch that earns its keep: it carries its name as a bare
``str`` and emits no ``ast.Name`` node, so forcing ``_TYPE_VAR`` to ``()`` on
3.14.7 turns ``T`` and ``U`` into false findings. ``ast.TypeAlias`` is defensive
by comparison -- its name *is* an ``ast.Name`` in ``Store`` context, which the
preceding branch already catches -- and is left as it stands rather than removed.
A new ``test_a_pep695_type_parameter_is_in_scope`` pins both the guards and the
behaviour, skipped below 3.12 because its fixture source cannot parse there.

Measured on real interpreters rather than simulated. 3.10.21 and 3.11.15:
5 failed, exit 1 -> 5 passed, 1 skipped, exit 0. 3.14.7: all 6 pass, exit 0.
133 passed over ``tests/project`` and ``tests/utilities/test_logging.py``, exit 0,
subtests unchanged at 487.

No changelog entry on this branch. Per the rule that no code branch touches
``CHANGELOG.md`` or anything under ``docs/source/changelog/``, this change's entry
-- and the wording correction the ``MANIFEST.in`` claim implies for the #619
entry, plus the missing ``(#570)`` and ``(#577)`` citations -- go to the shared
changelog pull request #657 instead.
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

None yet

Development

Successfully merging this pull request may close these issues.

1 participant