build(tox): give each linter its own envdir and bound the unpinned tools - #1326
build(tox): give each linter its own envdir and bound the unpinned tools#1326ryanhill1 wants to merge 1 commit into
Conversation
All six linter envs shared envdir = .tox/linters while declaring different deps. tox keys an envdir to the dep set that built it, so every switch between stages tore the directory down and rebuilt it: after running pylint, a re-run of headers spends 4.28s reinstalling where an isolated env spends 0.05s. The sharing also decides what each tool can see. `tox -e pylint` installs pylint alone, so pylint cannot import qbraid_core and never infers types from it. The pylint stage inside `tox -e format-check` runs with qbraid_core present, pulled in by the qbraid-cli that headers needs. Same command, two different inputs depending on which entry point you use, and a reused .tox/linters carries whichever set happened to land there last. That is enough for a local run to disagree with CI, which always builds the directory fresh. pylint, isort and mypy were unpinned while black and ruff carry upper bounds with comments explaining the incidents that put them there. An unpinned linter changes what CI enforces without a commit, so all five are bounded now. Current resolved versions are pylint 4.0.7, isort 8.0.1 and mypy 2.3.0, all inside the new bounds. Cost is disk: six small tool envs instead of one shared. Cold install is unchanged in CI, which starts empty either way, and warm local runs get faster because switching stages no longer reinstalls. format-check: 10.00/10 from a cold .tox.
Argus reviewAuto-review is off for this repo. Tick the box below to run a review on this PR.
Estimated cost
Tip: you can also comment |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
👋 Hey there! It looks like the changelog might need an update. Please take a moment to edit the
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
All six linter envs shared
envdir = .tox/linterswhile declaring different deps. tox keys an envdir to the dep set that built it, so switching stages tears the directory down and rebuilds it.Two consequences
Stages reinstall each other. Measured, running
headersafterpylint:Each tool's inputs depend on the entry point.
tox -e pylintinstalls pylint alone, so pylint cannot importqbraid_coreand never infers types from it. The pylint stage insidetox -e format-checkruns withqbraid_corepresent, pulled in by theqbraid-clithatheadersneeds:Same
pylint qbraid bin tests, two different dependency sets. A reused.tox/linterscarries whichever set landed there last, which is enough for a local run to disagree with CI, since CI always builds the directory fresh. I hit exactly that: a localformat-checkreporting fourE1101: ... has no 'status_code' membererrors that do not reproduce on a clean env or in CI.Also: bound the unpinned tools
pylint,isortandmypywere unpinned.blackandruffalready carry upper bounds with comments naming the incidents that put them there, most recently ruff 0.16.0 changing default rules and surfacing ~742 pre-existing issues repo-wide (#1296). An unpinned linter changes what CI enforces without a commit, so all five are bounded now.Resolved versions today are
pylint 4.0.7,isort 8.0.1,mypy 2.3.0, all inside the new bounds, so this pins the status quo rather than moving anything.Trade-off
Six small tool envs instead of one shared directory, so more disk locally. CI is unaffected on cold install since it starts empty either way, and warm local runs get faster because switching stages no longer reinstalls.
Verification
tox -e format-checkfrom a cold.tox: 10.00/10, all five stages pass. Theheaders -> pylint -> headerssequence reuses both envs instead of rebuilding.