Clarify commit message from pr-auto-commit#6706
Conversation
Sorry, something went wrong.
📝 WalkthroughWalkthroughUpdates the GitHub Actions PR auto-commit workflow to configure git, run formatting/check steps that may conditionally commit, refactors change detection to compare HEAD with the PR head SHA, and records committed commands; also reorders imports in Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Actions Runner
participant Git as Git CLI
participant Formatter as Ruff/cargo fmt
participant Script as generate_opcode_metadata.py
participant GitHub as GitHub API
Runner->>Git: configure user.name/email
Runner->>Git: checkout PR head
Runner->>Formatter: run cargo fmt
Formatter-->>Runner: formatted files
Runner->>Git: check for changes vs PR head SHA
alt changes detected
Runner->>Git: commit formatting changes
end
Runner->>Formatter: run ruff format
Formatter-->>Runner: formatted Python
Runner->>Git: check & commit if changed
Runner->>Formatter: run ruff import-sorting check
Formatter-->>Runner: check result
Runner->>Git: commit changes if any
Runner->>Script: run generate_opcode_metadata.py
Script-->>Runner: generated files
Runner->>Git: commit generated files if any
Runner->>GitHub: push to PR head ref
Runner->>GitHub: post PR comment listing committed commands
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Sorry, something went wrong.
|
Code has been automatically formatted The code in this PR has been formatted using git pull origin clarify-commit-message |
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/pr-auto-commit.yaml (1)
100-107: Update the PR comment to reflect all auto-formatting steps.The comment message only mentions
cargo fmt --all, but the workflow now potentially applies multiple types of changes (cargo fmt, ruff format, ruff check import sorting, and generate_opcode_metadata.py). This could mislead contributors about what was actually changed.💬 Proposed fix for the PR comment message
message: | **Code has been automatically formatted** - The code in this PR has been formatted using `cargo fmt --all`. + The code in this PR has been automatically formatted and/or updated. + This may include: `cargo fmt --all`, `ruff format`, import sorting, and generated code updates. Please pull the latest changes before pushing again: ```bash git pull origin ${{ github.event.pull_request.head.ref }}
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/pr-auto-commit.yamlscripts/fix_test.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: In most cases, Python code should not be edited; bug fixes should be made through Rust code modifications only
Follow PEP 8 style for custom Python code
Use ruff for linting Python code
Files:
scripts/fix_test.py
**/*test*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*test*.py: NEVER comment out or delete any test code lines except for removing@unittest.expectedFailuredecorators and upper TODO comments
NEVER modify test assertions, test logic, or test data in test files
When a test cannot pass due to missing language features, keep it as expectedFailure and document the reason; do not comment it out
The only acceptable modifications to test files are: (1) Removing@unittest.expectedFailuredecorators and the upper TODO comments when tests actually pass, and (2) Adding@unittest.expectedFailuredecorators when tests cannot be fixed
Files:
scripts/fix_test.py
🧠 Learnings (5)
📓 Common learnings
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.
📚 Learning: 2025-08-30T14:40:05.858Z
Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 6089
File: scripts/lib_updater.py:292-297
Timestamp: 2025-08-30T14:40:05.858Z
Learning: In scripts/lib_updater.py, the --inplace flag intentionally writes to orig_file (not remote_file) even though patches are applied to remote_file content. This workflow allows updating the original RustPython test file with patches applied to new upstream CPython content.
Applied to files:
scripts/fix_test.py
📚 Learning: 2025-12-27T14:03:49.034Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: Applies to **/*test*.py : The only acceptable modifications to test files are: (1) Removing `unittest.expectedFailure` decorators and the upper TODO comments when tests actually pass, and (2) Adding `unittest.expectedFailure` decorators when tests cannot be fixed
Applied to files:
scripts/fix_test.py
📚 Learning: 2025-12-27T14:03:49.034Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
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
📚 Learning: 2025-12-27T14:03:49.034Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: Applies to **/*.rs : Always run clippy to lint code (`cargo clippy`) before completing tasks and fix any warnings or lints introduced by changes
Applied to files:
.github/workflows/pr-auto-commit.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Run rust tests (windows-2025)
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Run snippets and cpython tests (windows-2025)
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Run rust tests (ubuntu-latest)
- GitHub Check: Check Rust code with clippy
- GitHub Check: Run tests under miri
- GitHub Check: Check the WASM package and demo
- GitHub Check: Ensure compilation on various targets
🔇 Additional comments (8)
scripts/fix_test.py (1)
30-30: LGTM! Import sorting aligns with the new workflow.This alphabetical reordering of imports is consistent with the
ruff check --select I --fixstep introduced in the pr-auto-commit workflow..github/workflows/pr-auto-commit.yaml (7)
37-40: LGTM! Explicit git configuration improves clarity.Adding this dedicated step ensures consistent committer identity for all auto-commits and makes the workflow more maintainable.
46-49: LGTM! Conditional commit achieves the PR objective.The workflow now only creates a "cargo fmt" commit when formatting actually makes changes, addressing the core issue described in the PR.
57-63: LGTM! Separate commit for ruff format improves clarity.This separate step with its own conditional commit allows distinguishing between formatting changes and import sorting changes in the commit history.
65-71: LGTM! Import sorting as a separate step provides clarity.The separate commit for import sorting makes it clear what type of auto-formatting was applied, and explains the changes in scripts/fix_test.py.
73-79: LGTM! Separate commit for code generation is appropriate.The distinct "Auto-generate" prefix in the commit message clearly differentiates generated code changes from formatting changes.
84-84: LGTM! Improved change detection logic.Comparing HEAD to the original PR head SHA correctly detects whether any of the conditional formatting/generation steps created commits, which is more robust than checking the working directory state.
90-93: LGTM! Step name accurately reflects its purpose.The renamed step correctly indicates it only pushes commits made by previous steps, not creating new commits itself.
Sorry, something went wrong.
moreal
force-pushed
the
clarify-commit-message
branch
from
61bc77e to
a5a6ca1
Compare
January 11, 2026 19:18
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.github/workflows/pr-auto-commit.yaml:
- Around line 95-99: The git push step uses an unquoted expression `${{
github.event.pull_request.head.ref }}` which actionlint flags for possible
script injection; fix by passing the PR ref via an environment variable and
quoting it in the push command: set an env key (e.g., PR_REF) to `${{
github.event.pull_request.head.ref }}` on the "Push formatting changes" step and
replace the direct expression in the run command with a quoted reference to that
env var (e.g., git push origin HEAD:"${PR_REF}"), ensuring the ref is properly
quoted to prevent shell injection.
- Around line 77-85: The "Run generate_opcode_metadata.py" step runs a
PR-controlled script and can exfiltrate AUTO_COMMIT_PAT for forked PRs; guard
that step so it does not run for forked repositories by adding a condition that
checks the PR head repo is not a fork (e.g., use
github.event.pull_request.head.repo.fork == false) before executing python
scripts/generate_opcode_metadata.py and the git commit logic, leaving the step
intact for non-fork PRs only.
- Around line 37-42: The workflow currently uses echo "" to initialize
/tmp/committed_commands.txt which writes a leading blank line; replace that echo
call with a true truncation/no-op redirection so the file is empty without a
newline (e.g., use a plain shell redirection or printf with an empty string) and
keep the file name /tmp/committed_commands.txt unchanged.
- Around line 43-85: The commit steps currently stage with git add -u (which
ignores untracked files) but gate on git status --porcelain (which sees
untracked), causing git commit to fail when only new files exist; for each of
the four commit blocks (the cargo fmt, ruff format, ruff check --select I --fix,
and generate_opcode_metadata.py blocks) replace git add -u with git add -A so
untracked files are included before committing, ensuring git commit succeeds
when new files were created.
🧹 Nitpick comments (1)
.github/workflows/pr-auto-commit.yaml (1)
86-94:has_changesdetection is OK if all changes are committed; consider asserting a clean working tree.
This check only detects new commits (HEAD change). If anything leaves the tree dirty, the workflow will silently skip pushing.Suggestion: fail fast if the working tree isn’t clean before computing
has_changes.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/pr-auto-commit.yamlscripts/fix_test.py
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/fix_test.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
📚 Learning: 2025-12-27T14:03:49.034Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
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
🪛 actionlint (1.7.10)
.github/workflows/pr-auto-commit.yaml
97-97: "github.event.pull_request.head.ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Run snippets and cpython tests (windows-2025)
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Check Rust code with clippy
- GitHub Check: Run rust tests (windows-2025)
- GitHub Check: Run rust tests (ubuntu-latest)
- GitHub Check: Check the WASM package and demo
- GitHub Check: Ensure compilation on various targets
🔇 Additional comments (1)
.github/workflows/pr-auto-commit.yaml (1)
100-107: Nice: PR comment now reflects what actually got committed.
The recorded command list makes the message accurate and reduces confusion.Also applies to: 113-117
Sorry, something went wrong.
ShaharNaveh
left a comment
ShaharNaveh
left a comment
There was a problem hiding this comment.
looks great!
Sorry, something went wrong.
youknowone
left a comment
youknowone
left a comment
There was a problem hiding this comment.
Thank you!
Sorry, something went wrong.
youknowone
merged commit f3b7f7f
into
RustPython:main
Jan 11, 2026
* Clarify commit message from pr-auto-commit * Auto-format: cargo fmt --all --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Reviewers
3 participants
The existing pr-auto-commit workflow did not perform separate commits, so even though
cargo fmt --allwasn't actually executed, the commit message remained asAuto-format: cargo fmt --all, creating some potential confusion. This improves that issue.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.