Optimize CI cache usage by fanninpm · Pull Request #6707

Optimize CI cache usage by fanninpm · Pull Request #6707 · RustPython/RustPython

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In @.github/workflows/ci.yaml:
- Around line 422-429: The sed invocation in the CI job names "Check whats_left
is not broken" is incorrectly quoted so it sees a leading double-quote in the
value `"${{ env.CARGO_ARGS }}"`, preventing the pattern `^--` from matching;
update the run lines that call whats_left.py to remove the escaped quotes and
use `${{ env.CARGO_ARGS }}` directly (so the sed expression s/^--[^ ]*// works),
or alternatively change the sed pattern to handle optional leading quotes (e.g.,
s/^"*--[^ ]*//), ensuring both the non-macOS and macOS steps that call python -I
whats_left.py use the fixed form.

🧹 Nitpick comments (1)

.github/workflows/ci.yaml (1)

426-426: Consider creating a tracking issue for the macOS JIT fix.

The TODO comment documents the macOS JIT exclusion, but this workaround might be forgotten over time. Consider opening a GitHub issue to track resolving the underlying JIT issue on macOS.

Would you like me to help draft an issue to track this?

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 29bb8b4 and f88cd49.

📒 Files selected for processing (2)

.github/workflows/ci.yaml
scripts/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.expectedFailure decorators 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.expectedFailure decorators and the upper TODO comments when tests actually pass, and (2) Adding @unittest.expectedFailure decorators when tests cannot be fixed

Files:

scripts/fix_test.py

🧠 Learnings (3)

📚 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/ci.yaml

📚 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-09-07T05:38:31.690Z

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 6089
File: scripts/lib_updater.py:198-202
Timestamp: 2025-09-07T05:38:31.690Z
Learning: In scripts/lib_updater.py, the iter_patches function intentionally does not handle SyntaxError from ast.parse(contents). The author confirmed this behavior is fine and intended - the tool should fail fast on unparseable files rather than silently skip processing.

Applied to files:

scripts/fix_test.py

🔇 Additional comments (1)

scripts/fix_test.py (1)

30-30: LGTM!

The import reordering follows alphabetical convention and is consistent with ruff formatting mentioned in the PR discussion.