Ruff as part of PR auto-format - #6421
Conversation
Sorry, something went wrong.
WalkthroughTwo GitHub Actions workflow files are modified to enhance Ruff linter integration. The CI workflow adds a version display flag to the Ruff action, while the PR auto-commit workflow introduces new Ruff format and import check steps into the code formatting pipeline. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. ❤️ ShareComment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1).github/workflows/pr-auto-commit.yaml (1)📜 Review details42-50: Auto-format now includes Ruff, but commit/PR message still says only cargo fmt --all.
Since ruff format and ruff check --select I --fix can change files, the commit message (Line 69) and sticky PR comment text (Line 80) will be misleading when Ruff makes edits. Consider updating them to mention Ruff as well (or use a generic “Auto-format” message).
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📥 CommitsReviewing files that changed from the base of the PR and between 49522e7 and 2275ef2.
📒 Files selected for processing (2)- .github/workflows/ci.yaml (1 hunks)
- .github/workflows/pr-auto-commit.yaml (1 hunks)
Learnt from: CR Repo: RustPython/RustPython PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-11-29T12:17:28.606Z Learning: Applies to **/*.py : Use ruff for linting Python code
Learnt from: ShaharNaveh Repo: RustPython/RustPython PR: 5932 File: .github/workflows/comment-commands.yml:18-24 Timestamp: 2025-07-10T10:08:43.330Z Learning: In GitHub Actions workflows for the RustPython project, the maintainer ShaharNaveh prefers to keep workflows simple and doesn't mind if steps fail when the desired state is already achieved (e.g., user already assigned to an issue). Avoid suggesting complex error handling for edge cases they don't consider problematic.
Learnt from: CR Repo: RustPython/RustPython PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-11-29T12:17:28.606Z Learning: Applies to **/*.py : Use ruff for linting Python code
Applied to files:
- .github/workflows/ci.yaml
- .github/workflows/pr-auto-commit.yaml
Learnt from: CR Repo: RustPython/RustPython PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-11-29T12:17:28.606Z Learning: Applies to **/*.rs : Always run clippy to lint Rust code (`cargo clippy`) before completing tasks and fix any warnings or lints introduced by changes
Applied to files:
- .github/workflows/pr-auto-commit.yaml
Learnt from: CR Repo: RustPython/RustPython PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-11-29T12:17:28.606Z Learning: Applies to **/*.rs : Follow the default rustfmt code style by running `cargo fmt` to format Rust code
Applied to files:
- .github/workflows/pr-auto-commit.yaml
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Check the WASM package and demo
- GitHub Check: Run rust tests (windows-latest)
- GitHub Check: Check Rust code with clippy
- GitHub Check: Run rust tests (ubuntu-latest)
- GitHub Check: Run snippets and cpython tests on wasm-wasi
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Ensure compilation on various targets
.github/workflows/ci.yaml (1)321-330: Ruff installation step is correctly configured — action installs ruff on PATH despite args: "--version".
The args: "--version" parameter tells the action to only print the version after installing, rather than running linting checks. The action still installs ruff to PATH regardless of the args value, so subsequent steps (ruff check --diff and ruff format --check) will execute correctly. The version: "0.14.9" pin is properly specified and will be applied.
Sorry, something went wrong.
| - run: ruff format | ||
| - run: ruff check --select I --fix | ||
|
|
There was a problem hiding this comment.
⚠️ Potential issue | 🟠 Major
🧩 Analysis chain🌐 Web query:
ruff import sorting affects formatting stability ruff format after ruff check I fix
💡 Result:
Short answer: Ruff separates concerns — import sorting is a lint fix (rule I001) and the formatter intentionally does not reorder imports. Run the linter fixer first, then the formatter:
- ruff check --select I --fix .
- ruff format .
This ensures imports are grouped/sorted (I rules) and then the file is formatted. You can automate via a pre-commit hook or add select/settings in ruff.toml. [1][2][3]
Sources:
- Ruff formatter docs (notes import-sorting is done by the linter). [1]
- unsorted-imports (I001) rule (describes de-dup/group/sort behavior). [2]
- Community / how-to references (Stack Overflow / guides showing the two-step sequence). [3]
Swap the order: run ruff check --select I --fix before ruff format for stable formatting.
Ruff separates import sorting (a linter concern) from formatting. Running import fixes after formatting can undo formatting changes, since the formatter does not reorder imports. The recommended sequence is ruff check --select I --fix . followed by ruff format ..
🤖 Prompt for AI Agents.github/workflows/pr-auto-commit.yaml lines 48-50: the workflow runs ruff format before running ruff check --select I --fix which can cause import-fix changes to be overwritten; swap the commands so ruff check --select I --fix (ideally with a path like `.`) runs first to apply import sorting, then run ruff format (also with `.`) to produce stable formatting; update the two run steps accordingly and ensure both commands target the repository path.
Sorry, something went wrong.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.