Uh oh!
There was an error while loading. Please reload this page.
Stop using a string literal as a format argument - #96248
Conversation
rust-highfive
commented
Apr 20, 2022
(rust-highfive has picked a reviewer for you, use r? to override) |
compiler-errors
commented
Apr 20, 2022
@TaKO8Ki, one line PRs like this are (in my opinion) not significant enough to review... Performance hasn't changed and readability only increased slightly. Do you mind including changes like this in a more significant PR when you're editing code near by, instead of putting up such small changes independently? Thanks! |
TaKO8Ki
commented
Apr 20, 2022
@compiler-errors |
compiler-errors
commented
Apr 20, 2022
I am just confused why not all of the usages were fixed in this file, or even (if i saw correctly) in this function? I recommend batching at least a few of these into a PR, so we don't have a lot of tiny commits in the logs.. |
TaKO8Ki
commented
Apr 21, 2022
@compiler-errors - format!("{}{}", snippet, ".collect::<Vec<_>>()"),+ format!("{}.collect::<Vec<_>>()", snippet),Should I make additional changes like #96065? |
compiler-errors
commented
Apr 21, 2022
I am still not convinced that this change does anything meaningful, but also I will not block it. Thanks for making this contribution -- but also keep this in mind when putting up one-line changes like this that don't affect performance or readability 😅 If you want to change more spots to use format-args-capture, then at least please batch it together, maybe a few crates together at once. @bors r+ rollup=always |
bors
commented
Apr 21, 2022
📌 Commit 5078b05 has been approved by |
TaKO8Ki
commented
Apr 21, 2022
@compiler-errors |
Rollup of 5 pull requests Successful merges: - rust-lang#95434 (Only output DepKind in dump-dep-graph.) - rust-lang#96248 (Stop using a string literal as a format argument) - rust-lang#96251 (Update books) - rust-lang#96269 (errors: minor translation-related changes) - rust-lang#96289 (Remove redundant `format!`s) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
No description provided.