test_auto_format 02 #6249
test_auto_format 02 #6249
Conversation
WalkthroughMinor formatting changes applied to Windows resource handling in build.rs. The else branch that logs missing logo.ico is condensed to a single line, and the chained res.compile() call within map_err is reformatted to a single line. No functional or control flow changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
|
Code has been automatically formatted The code in this PR has been formatted using Triggered by commit: You may need to pull the latest changes before pushing again: git pull origin test |
|
@fanninpm this? |
![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
🧹 Nitpick comments (1)
build.rs (1)
10-13: Consider more idiomatic error handling pattern.The
map_err(|e| {...}).ok()pattern is unconventional for errors that are being logged but discarded. Consider more explicit alternatives such aslet _ = res.compile().map_err(|e| println!(...));or adding a comment explaining why the error is intentionally ignored.Apply this diff for more idiomatic error handling:
- res.compile().map_err(|e| { - println!("cargo:warning=Failed to compile Windows resources: {e}"); - }) - .ok(); + let _ = res.compile().map_err(|e| { + println!("cargo:warning=Failed to compile Windows resources: {e}"); + });Alternatively, if the intent is to never fail the build despite resource compilation errors, add a comment clarifying this design decision.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
build.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Format Rust code with the default rustfmt style (runcargo fmt)
Run clippy and fix any warnings or lints introduced by your changes
Follow Rust best practices for error handling and memory management
Files:
build.rs
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to Lib/test/**/*.py : When modifying CPython tests, add a '# TODO: RUSTPYTHON' comment documenting the reason
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Run clippy and fix any warnings or lints introduced by your changes
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Follow Rust best practices for error handling and memory management
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Format Rust code with the default rustfmt style (run `cargo fmt`)
📚 Learning: 2025-09-11T05:06:59.971Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Run clippy and fix any warnings or lints introduced by your changes
Applied to files:
build.rs
📚 Learning: 2025-09-11T05:06:59.971Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Follow Rust best practices for error handling and memory management
Applied to files:
build.rs
⏰ 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). (1)
- GitHub Check: auto_format
🔇 Additional comments (2)
build.rs (2)
7-9: Formatting does not align with standard rustfmt style.Line 7 combines the opening brace and
printlnon the same line, but placesreturnon the next line. This is inconsistent with standard Rust formatting conventions. Standardrustfmtwould either place the entire block contents on separate lines or condense the entire construct.Please run
cargo fmtand verify the output matches the standard formatting style. As per coding guidelines, the code should follow the default rustfmt style.
1-15: I cannot executecargocommands in this sandbox environment due to system limitations. Since the script failed to run, I need to request manual verification.Manual verification required for clippy compliance
The shell script failed to execute due to sandbox environment constraints. Please run the following commands locally to verify the code meets all clippy requirements:
cargo fmt --check build.rs cargo clippy --all-targets -- -D warningsAdditionally, review the code pattern at lines 10-13 where
map_err(...).ok()is used—this pattern converts the Result to an Option and discards it. Verify that no clippy lints are triggered by this error suppression approach.
I guess I was wrong. |
|
but there will be some edge cases that i not aware of |
2 participants
Summary by CodeRabbit