Skip to content

chore: add isort to Pipfile's dev-packages so make isort resolves it - #812

Merged
JarryShaw merged 1 commit into
mainfrom
fix/810-pipfile-isort-dev-package
Sep 25, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/810-pipfile-isort-dev-package

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • chore — anything else

Description

make isort drives its four recipe lines through pipenv run, which resolves against Pipfile's [dev-packages] — where isort was missing, though mypy, pylint and bandit all sit there. #786 put isort on the test extra only, so a pipenv install --dev environment could not run the recipe.

Added isort = "*" between bandit and mypy, bare like its neighbours. Additive only: the pyproject.toml extra serves consumers of the published package, Pipfile this repo's dev environment — nothing removed from either.

tests/project/test_isort_clean.py's docstring asserted the gap was still open ("isort remains absent from Pipfile"); this change makes that stale, so it is corrected.

Audit of every Makefile-invoked executable vs [dev-packages] — isort was the only omission:

Tool Makefile Pipfile
isort 133–136 was missing → added
build / twine 63 / 66–68 present
pytest / coverage 88, 92, 121 / 95–96 present
sphinx / sphinx-autobuild 124 / 130 via docs/Makefile present
vermin / pylint / mypy / bandit 182 / 185 / 188 / 191 present
pcapkit-vendor 78 present, via editable pypcapkit

Pipfile.lock is untracked (.gitignore:37, absent from disk) and make pipenv uses --skip-lock, so no lock refresh applies. Noted in passing, not fixed here: profile: (194) delegates to test/, which no longer exists.

Verified: tests/project/ 127 passed / 490 subtests, with test_isort_clean running for real (4 subtests, not skipped); tests/test_tier_guard.py 102 passed / 540 subtests. No test reads Pipfile from disk, so the new line changes no test's behaviour.

Closes #810

`make isort` runs its four recipe lines through `pipenv run`, which resolves
against `Pipfile`'s `[dev-packages]` -- and isort was not listed there, next to
the `mypy` and `pylint` it sits beside in the Makefile. So a `pipenv install
--dev` environment could not run the recipe, and #786's fix put isort on the
`test` extra only, leaving the two declarations disagreeing about a tool the
Makefile needs.

- Pipfile: add `isort = "*"` between `bandit` and `mypy`, bare like its
  neighbours. Additive only -- the `pyproject.toml` extra serves consumers of
  the published package, `Pipfile` this repo's dev environment.
- tests/project/test_isort_clean.py: its docstring recorded the gap as still
  open ("isort remains absent from Pipfile"), which this change makes stale.

Audited every executable the Makefile invokes against `[dev-packages]`: isort
was the only omission. build, twine, pytest, coverage, sphinx,
sphinx-autobuild, vermin, pylint, mypy and bandit are all present.

Closes #810
@JarryShaw JarryShaw added chore Maintenance work: tooling, repo hygiene, no library behaviour change dependencies Pull requests that update a dependency file python Pull requests that update Python code test Pull requests that add or correct tests (test: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Sep 25, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict: GOOD TO GO (sonnet; author was haiku). Every claim re-derived independently, nothing found inaccurate.

The Makefile audit reproduces exactly — isort was the only omission. The reviewer read the whole Makefile rather than grepping, and confirmed the trap the author flagged: grep -n "pipenv run" Makefile returns 20 lines but silently misses vermin, vermin-ci, pylint, mypy, bandit at :172, :182, :185, :188, :191, which route through $(RUN) (Makefile:144, RUN ?= pipenv run). Caught with grep -n '\$(RUN)' as the negative-catching probe. All eleven other tools present, including pcapkit-vendor as a console script of the editable install (pyproject.toml:107).

The docstring rewrite is necessary, not scope creep. The old prose said "isort remains absent from Pipfile… Out of scope here" — fixing #810 makes that a false statement in the tree, so leaving it would have been worse than the edit. The new text states a durable structural fact (which file make isort reads versus which serves published-package consumers) rather than something that drifts.

Pipfile.lock: all three sub-claims confirmed — .gitignore:37 is exactly Pipfile.lock, the file is absent from disk, and Makefile:75 is pipenv install --skip-lock --dev. So no lock refresh arises and none was performed.

The skip-vs-run distinction was verified rather than trusted, which is the right instinct since a silent skip is the exact defect #810 is about: HAS_ISORT evaluates True, _isort_recipe_line_numbers() returns exactly 4 entries matching MAKEFILE_LINES, so four real isort --check-only subprocesses run. And it self-tested the gate by monkeypatching find_spec to return None, confirming HAS_ISORT flips to False — proving the gate distinguishes ran from skipped rather than always reporting the same thing. tests/project/ 127 passed / 490 subtests, matching the author exactly.

The two flagged non-fixes confirmed and correctly left out: Makefile:193-194's profile: delegates to $(MAKE) -C test profile while test/ has zero tracked files and does not exist — a dead target; and snakeviz/tuna sit in [dev-packages] with no recipe touching them. Both pre-existing and orthogonal to #810; a follow-up, not a blocker here.

Flipping to review: good-to-go.

@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 Sep 25, 2026
@JarryShaw
JarryShaw merged commit 98b15aa into main Sep 25, 2026
31 checks passed
@JarryShaw
JarryShaw deleted the fix/810-pipfile-isort-dev-package branch September 25, 2026 22:25
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance work: tooling, repo hygiene, no library behaviour change dependencies Pull requests that update a dependency file python Pull requests that update Python code test Pull requests that add or correct tests (test: subject prefix)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore: Pipfile's dev-packages omits isort, which make isort and test_isort_clean both need

1 participant