Declare ruff's vendored-file excludes once in pyproject.toml - #9090
Declare ruff's vendored-file excludes once in pyproject.toml#9090hjmjohnson wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughRuff now excludes Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to Ruff exclusions are centralized in the shared configuration while the test script removes duplicate flags, with no remaining merge-readiness risk identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description gives clear context, implementation details, scope, verification results, and related PR information. It does not reproduce the template headings or checklist, but it contains the required change description and verification context. Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
a82b9a2 to
7839e1c
Compare
versioneer.py and monai/_version.py were excluded from ruff twice, independently: .pre-commit-config.yaml's ruff hook via its own "exclude:" regex, and runtests.sh via hardcoded --exclude flags on the CLI invocation. Neither read from pyproject.toml, so a third direct "ruff check" invocation (an editor, a one-off shell command) would lint and offer to rewrite both files -- 200 violations today. extend-exclude in [tool.ruff] gives runtests.sh's directory-walk invocation (and any other caller that lets ruff discover its own config) the same exclusion pre-commit's hook already applies, so the CLI flags on the runtests.sh invocation are redundant and dropped. Verified: 'ruff check' with pyproject.toml's new extend-exclude, given the directory the way runtests.sh gives it, still reports 'All checks passed!' for the tree, and 'runtests.sh --ruff' is unaffected end to end. Ruff ignores config exclude when handed explicit filenames instead of a directory (the pre-commit hook path, already governed by its own hook-level exclude, and not something this PR touches). Signed-off-by: Hans Johnson <hans-johnson@uiowa.edu>
7839e1c to
02eb3d5
Compare
1 participant
versioneer.pyandmonai/_version.pywere excluded from ruff twice, independently:.pre-commit-config.yaml's hook via its ownexclude:regex, andruntests.shvia hardcoded--excludeflags on the CLI invocation. Neither read frompyproject.toml, so a third directruff checkinvocation — an editor, a one-off shell command — would lint and offer to rewrite both files: 200 violations today.extend-excludein[tool.ruff]givesruntests.sh's directory-walk invocation the same exclusion pre-commit's hook already applies, so the CLI flags there are now redundant and dropped.Independent of #9089 (which routes the same block's
ruffcalls throughPY_EXE) — rebuilt directly offdevso the two PRs' diffs don't overlap. Whichever lands first, the other needs a small rebase over the same lines; neither depends on the other's content.Verification
ruff checkgiven the directory the wayruntests.shgives it (not explicit filenames) reportsAll checks passed!for the tree with the newextend-excludein place, andruntests.sh --ruffis unaffected end to end.--excludeflag being dropped was purely additive overextend-exclude, not required for it to take effect: a syntheticpyproject.tomlwith onlyextend-excludeset already hid the excluded file fromruff check ., before any--excludeflag was added.exclude/extend-excludewhen handed explicit filenames instead of a directory — that's the pre-commit hook's path, already governed by its own hook-levelexclude:, and this PR doesn't touch it.