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
@JarryShaw JarryShaw added this to the 1.5 milestone Oct 6, 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

Status: Done

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