Skip to content

clippy: fix uninlined format args #287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

ValuedMammal
Copy link
Collaborator

Fix #281 by taking clippy's suggestion to inline the arguments to a format! string where applicable.

Also update rust-version to 1.88.0, closing #280.

Notes to the reviewers

I've gone ahead and fixed the clippy warnings, although some have argued that the lint is quite pedantic and not entirely helpful when it comes to style and readability. Therefore I also allowed the lint to prevent clippy from warning about it in the future d75126e. That change can be reverted if/when rust lang moves uninlined_format_args back to the pedantic, i.e. not default group of lints rust-lang/rust-clippy#15287.

Checklists

All Submissions:

The lint should have stayed in "pedantic". The lint itself isn't
helpful because it doesn't always improve readability, and
inlining won't work if the argument is an expression or field
access.
@ValuedMammal ValuedMammal self-assigned this Jul 31, 2025
@ValuedMammal ValuedMammal added this to the Wallet 2.1.0 milestone Jul 31, 2025
@ValuedMammal ValuedMammal moved this to Needs Review in BDK Wallet Jul 31, 2025
@ValuedMammal ValuedMammal added the chore Non-coding related work label Jul 31, 2025
@coveralls
Copy link

Pull Request Test Coverage Report for Build 16654683048

Details

  • 7 of 48 (14.58%) changed or added relevant lines in 9 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.01%) to 84.747%

Changes Missing Coverage Covered Lines Changed/Added Lines %
wallet/src/descriptor/policy.rs 1 3 33.33%
wallet/src/keys/mod.rs 0 3 0.0%
wallet/src/wallet/mod.rs 0 3 0.0%
wallet/src/wallet/signer.rs 0 4 0.0%
wallet/src/wallet/persisted.rs 0 7 0.0%
wallet/src/descriptor/error.rs 0 8 0.0%
wallet/src/wallet/error.rs 0 14 0.0%
Files with Coverage Reduction New Missed Lines %
wallet/src/wallet/persisted.rs 1 36.99%
Totals Coverage Status
Change from base Build 16389875164: 0.01%
Covered Lines: 6573
Relevant Lines: 7756

💛 - Coveralls

@@ -7,6 +7,7 @@
)]
#![no_std]
#![warn(missing_docs)]
#![allow(clippy::uninlined_format_args)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't mind leaving this lint enabled even if it's a bit pedantic. But no strong feelings about it either way so OK to allow for now.

Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK d75126e

You'll need to update the master branch rules settings to version 1.88.0 before merging.

Copy link
Contributor

@oleonardolima oleonardolima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK d75126e

@notmandatory notmandatory merged commit c39ce79 into bitcoindevkit:master Jul 31, 2025
20 checks passed
@github-project-automation github-project-automation bot moved this from Needs Review to Done in BDK Wallet Jul 31, 2025
@ValuedMammal ValuedMammal deleted the clippy/inline_fmt_args branch August 9, 2025 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Non-coding related work
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[clippy] Fix uninlined_format_args
4 participants