Skip to content

Fix clippy lints in integration_test#261

Merged
tcharding merged 7 commits intorust-bitcoin:masterfrom
jamillambert:0620-lint-tests
Jun 24, 2025
Merged

Fix clippy lints in integration_test#261
tcharding merged 7 commits intorust-bitcoin:masterfrom
jamillambert:0620-lint-tests

Conversation

@jamillambert
Copy link
Collaborator

A new script to lint the integration tests is being added in #258.

Run the script and fix all the lint errors.

@jamillambert jamillambert changed the title Fix clippy lints Fix clippy lints in integration_test Jun 20, 2025
@tcharding
Copy link
Member

All the changes that remove let _ = should have an explicit type. Either like #259 for null or whatever version specific type the return type is.

Its ok this time but doing all the lints in a single patch costs the reviewer brain space for the benefit of the dev. Review time is always more valuable than dev time in open source projects. In this case its no problem but just a thing to keep in mind as you advance in open source.

Ord::max already returns a u32, casting to u32 is redundant.

Remove `as u32` on the u32 value.
Lint error due to unnecessary `== false`, replace it with `!`
Some functions that take a value were passed a reference.

Change them all to pass the value.
The for loop can be written more simply.

Change it to iterate over `.values()`.
There is a conversion of sats to btc that uses groupings of 100_000_000.

Allow this inconsistent digit grouping to aid in readability and remove
the lint error.
The assert checking if the key is in the map can be written more simply.

Use the `contains_key` function to make the code clearer and remove the
lint error.
Two cases of rpcs that return null but were assigned to a variable `_`
caused a lint error.

Declare the variable as a unit type for the return value of rpcs that
return null.
@jamillambert
Copy link
Collaborator Author

I have split it into individual commits for each type of lint error.

The only change is the last patch that specifically assigns the return of the rpcs that return null to a unit type variable, rather than just dropping them.

Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK 449f319

@tcharding tcharding merged commit b1eb82b into rust-bitcoin:master Jun 24, 2025
29 checks passed
blaze-smith470pm added a commit to blaze-smith470pm/corepc that referenced this pull request Sep 26, 2025
449f319e408f77c93cf2a90acfdf645d9b7d4787 Add let _: () = ... to rpc calls that return null (Jamil Lambert, PhD)
1810527de409d8c99d877a48115fbdc489e86fcc Simplify assert on map key (Jamil Lambert, PhD)
acd9332afb7a13104474bcd96bf79abec2905264 Allow inconsistent digit grouping (Jamil Lambert, PhD)
b5777a78250727f6f6297ab7c832ff9f27dc1782 Refactor for loop declaration (Jamil Lambert, PhD)
aabdaa3b644245eb4cb6beb6b572efdf63a1eb60 Pass value instead of reference (Jamil Lambert, PhD)
71824cd6f02db2de45139abb476b677f6ed82ff4 change == false to ! (Jamil Lambert, PhD)
fd8fa74065a11ae752a2fad555ccdaba1349523f Remove redundant as u32 (Jamil Lambert, PhD)

Pull request description:

  A new script to lint the integration tests is being added in #258.

  Run the script and fix all the lint errors.

ACKs for top commit:
  tcharding:
    ACK 449f319e408f77c93cf2a90acfdf645d9b7d4787

Tree-SHA512: 65f3429d227b05e4e54e252afb8b406100e743c1048fa05b86455d7020815b4966b3b1a2b291fb0c2064022b01dc90aed9a6f054a78b1840c81865bf42776adb
@jamillambert jamillambert deleted the 0620-lint-tests branch December 29, 2025 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants