Name the examples directories apart; fix the HoleDescriptor typo - #350
Merged
Merged
Conversation
examples/sample/ and examples/samples/ differed by one letter and held completely different things, so the pair read as a typo in every path that mentioned either. The captures now live in examples/captures/ and the scripts that build them in examples/generators/. Updated every reader: the legacy_smoke scripts, tests/_support.sample_path(), the Makefile targets, the CI workflow, README and the Help Wanted page. Full suite: 334 passed, 70 subtests passed.
The TCP hole descriptor data model was named HoleDiscriptor, and the misspelling had spread to the package alias, the docs and the tests. Renamed the class to HoleDescriptor and the package alias to TCP_HoleDescriptor, with no compatibility alias left behind: the old spelling is gone from the public API, so anything importing HoleDiscriptor or TCP_HoleDiscriptor must be updated.
There was a problem hiding this comment.
🟡 Changes recommended
The repo still contains examples/sample/ but .gitignore/MANIFEST.in no longer account for it, which can lead to stale fixtures being accidentally staged or packaged.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR performs naming cleanups across the repo: it splits example fixture data vs fixture generator scripts into clearly named directories, and fixes the long-standing HoleDiscriptor typo by renaming the TCP reassembly model to HoleDescriptor (including the package-level alias), updating tests, docs, and CI accordingly.
Changes:
- Rename example paths to use
examples/captures/(fixtures) andexamples/generators/(fixture builders), updating tests/docs/CI/Makefile and legacy smoke scripts. - Rename TCP reassembly hole model
HoleDiscriptor→HoleDescriptor(andTCP_HoleDiscriptor→TCP_HoleDescriptor) and update usages in code/tests/docs. - Add/update committed sample artifact files under
examples/captures/consistent with the new directory naming.
File summaries
| File | Description |
|---|---|
| tests/foundation/reassembly/test_tcp.py | Updates imports/usages to HoleDescriptor. |
| tests/foundation/reassembly/data/test_models.py | Updates TCP model alias/import expectations to TCP_HoleDescriptor / HoleDescriptor. |
| tests/_support.py | Points sample fixture resolution and regen command at examples/captures/ + examples/generators/. |
| README.rst | Updates documentation references to new fixture/generator directories. |
| pcapkit/foundation/reassembly/tcp.py | Updates TCP reassembly implementation to use HoleDescriptor. |
| pcapkit/foundation/reassembly/data/tcp.py | Renames the TCP hole data model and updates exported names/types. |
| pcapkit/foundation/reassembly/data/init.py | Renames package alias to TCP_HoleDescriptor. |
| MANIFEST.in | Switches pruning from examples/sample to examples/captures. |
| Makefile | Updates samples + isort targets to the new generator directory and paths. |
| examples/legacy_smoke/test_trace.py | Updates legacy smoke script fixture/output paths to ../captures/.... |
| examples/legacy_smoke/test_time.py | Updates legacy smoke script fixture paths to ../captures/.... |
| examples/legacy_smoke/test_tcp.py | Updates legacy smoke script fixture/output paths to ../captures/.... |
| examples/legacy_smoke/test_stream.py | Updates legacy smoke script output/buffer paths to ../captures/.... |
| examples/legacy_smoke/test_stream_askpass.py | Updates legacy smoke script output/buffer paths to ../captures/.... |
| examples/legacy_smoke/test_reassembly.py | Updates legacy smoke script input/output paths to ../captures/.... |
| examples/legacy_smoke/test_profile.py | Updates legacy smoke script fixture paths to ../captures/.... |
| examples/legacy_smoke/test_perf.py | Updates legacy smoke script fixture/output paths to ../captures/.... |
| examples/legacy_smoke/test_pcapng.py | Updates legacy smoke script fixture/output paths to ../captures/.... |
| examples/legacy_smoke/test_ipv6.py | Updates legacy smoke script fixture/output paths to ../captures/.... |
| examples/legacy_smoke/test_ipv6_reasm.py | Updates legacy smoke script input/output paths to ../captures/.... |
| examples/legacy_smoke/test_ip_reasm.py | Updates legacy smoke script input/output paths to ../captures/.... |
| examples/legacy_smoke/test_http.py | Updates legacy smoke script fixture paths to ../captures/.... |
| examples/legacy_smoke/test_file.py | Updates legacy smoke script fixture paths to ../captures/.... |
| examples/legacy_smoke/test_extractor.py | Updates legacy smoke script fixture/output paths to ../captures/.... |
| examples/legacy_smoke/test_engine.py | Updates legacy smoke script fixture/output paths to ../captures/.... |
| examples/legacy_smoke/test_basic.py | Updates legacy smoke script fixture/output paths to ../captures/.... |
| examples/legacy_smoke/test_api.py | Updates legacy smoke script fixture/output paths to ../captures/.... |
| examples/legacy_smoke/test_analyse.py | Updates legacy smoke script fixture paths to ../captures/.... |
| examples/generators/pcapng.py | Updates generator docs/constants and embedded provenance strings to new paths. |
| examples/generators/pcap.py | Updates generator docs/constants/comments to write into examples/captures/. |
| examples/generators/make_samples.py | Updates top-level regeneration script to target examples/captures/. |
| examples/generators/legacy.py | Updates legacy fixture generator to write into examples/captures/. |
| examples/captures/pcapng.txt | Adds/updates committed sample output under the new captures directory. |
| examples/captures/out.txt | Adds/updates committed sample output under the new captures directory. |
| examples/captures/out.plist | Adds/updates committed sample output under the new captures directory. |
| examples/captures/out.json | Adds/updates committed sample output under the new captures directory. |
| examples/captures/in.pcap | Adds/updates committed sample capture under the new captures directory. |
| examples/captures/dhcp.pcapng | Adds/updates committed sample capture under the new captures directory. |
| docs/source/pep.rst | Updates fixture regeneration documentation to new generator path. |
| docs/source/pcapkit/foundation/reassembly/tcp.rst | Updates Sphinx autoclass reference to HoleDescriptor. |
| .gitignore | Updates ignore/unignore rules to the new captures directory. |
| .github/workflows/unit-tests.yml | Updates CI fixture regeneration step and comments to new generator path/fixture directory. |
Review details
- Files reviewed: 35/49 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Owner
Author
|
@copilot Fix the code for all comments in this review thread. When a review comment includes a suggested change, apply the suggestion exactly. Do not make changes beyond what is described in the linked review thread. |
Co-authored-by: JarryShaw <15666417+JarryShaw@users.noreply.github.com>
Contributor
Added |
JarryShaw
added
fix
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two naming cleanups, no behaviour change beyond the removed misspelling.
examples/sample/vsexamples/samples/They differed by one letter and held different things — captures in one, the scripts that build them in the other — so every path mentioning either read as a typo. Now:
examples/captures/— the.pcap/.pcapngfixtures (still gitignored bar the six committed ones)examples/generators/—make_samples.py,pcap.py,pcapng.py,legacy.pyEvery reader updated: the 21
examples/legacy_smoke/scripts,tests/_support.sample_path(), thesamples/test-all/coverage/isortMakefile targets, both CI jobs,README.rstanddocs/source/pep.rst.HoleDiscriptor→HoleDescriptorThe TCP hole descriptor data model was misspelt, and it had spread to the package alias (
TCP_HoleDiscriptor), the docs page and the tests.This is a breaking change and deliberately so — no compatibility alias is left behind, so anything importing
pcapkit.foundation.reassembly.data.tcp.HoleDiscriptororpcapkit.foundation.reassembly.data.TCP_HoleDiscriptorneeds updating. Nothing inside the repository still uses the old spelling.Test result
pytest -q→ 334 passed, 70 subtests passed, unchanged frommain.