rpc: Resolve Rust 1.88 clippy lints and format strings#7047
rpc: Resolve Rust 1.88 clippy lints and format strings#7047steviez merged 4 commits intoanza-xyz:masterfrom
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7047 +/- ##
=========================================
- Coverage 83.2% 83.2% -0.1%
=========================================
Files 853 853
Lines 374734 374735 +1
=========================================
- Hits 311896 311862 -34
- Misses 62838 62873 +35 🚀 New features to boost your workflow:
|
| code: ErrorCode::InvalidRequest, | ||
| message: format!( | ||
| "There already exists a plugin named {} loaded, while reloading {name}. Did not load requested plugin", | ||
| "There already exists a plugin named {} loaded, while reloading {name}. Did \ |
There was a problem hiding this comment.
This was before your formatting, but I don't really like that one is passed as argument and the other is embedded. Maybe move name out from {}?
There was a problem hiding this comment.
I'm undecided on where I stand with format strings like these. But for the sake of this PR, I'm inclined to leave it as is; I'm mostly just trying to get us to a point where we can turn on an extra lint.
There are a bunch of these in the codebase, so if we do want to change, I think it'd be good to get a loose decision and then tackle making the codebase uniform in a separate change(s)
| test_serde::<UiTransactionStatusMeta>(json_input, expected_json_output); | ||
|
|
||
| #[rustfmt::skip] | ||
| let json_input = "{\ |
There was a problem hiding this comment.
maybe using here r#""# would simplify it?
There was a problem hiding this comment.
Ahh yeah, I think that would be more readable then what we have. That being said, what we have is workable and I kind of want to push this to keep overall progress moving so I think I'm going to defer for now
KirillLykov
left a comment
KirillLykov
left a comment
There was a problem hiding this comment.
Generally, look good to me. Left some cosmetic comments
steviez
left a comment
steviez
left a comment
There was a problem hiding this comment.
Thanks for the review; agree there could be some more minor cleanup in the future but going to merge as-is as I'm mostly focused on the one lint at the moment
| code: ErrorCode::InvalidRequest, | ||
| message: format!( | ||
| "There already exists a plugin named {} loaded, while reloading {name}. Did not load requested plugin", | ||
| "There already exists a plugin named {} loaded, while reloading {name}. Did \ |
There was a problem hiding this comment.
I'm undecided on where I stand with format strings like these. But for the sake of this PR, I'm inclined to leave it as is; I'm mostly just trying to get us to a point where we can turn on an extra lint.
There are a bunch of these in the codebase, so if we do want to change, I think it'd be good to get a loose decision and then tackle making the codebase uniform in a separate change(s)
| test_serde::<UiTransactionStatusMeta>(json_input, expected_json_output); | ||
|
|
||
| #[rustfmt::skip] | ||
| let json_input = "{\ |
There was a problem hiding this comment.
Ahh yeah, I think that would be more readable then what we have. That being said, what we have is workable and I kind of want to push this to keep overall progress moving so I think I'm going to defer for now
- Add rustfmt::skip to several already well-crafted strings - Run `cargo clippy --fix --tests` with Rust 1.88.0 set in `rust-toolchain.toml` - Run `cargo fmt` with `format_strings = true` set in `rustfmt.toml`
3 participants
Problem
Working towards #6850, broken out from #6854
Summary of Changes
cargo clippy --fix --testswith Rust 1.88.0 set inrust-toolchain.tomluninlined_format_argscargo fmtwithformat_strings = trueset inrustfmt.toml