Skip to content
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

Do clippy improvements #1367

Merged
merged 4 commits into from Nov 4, 2022
Merged

Conversation

tcharding
Copy link
Member

Clippy has been updated and new warnings are being triggered in our codebase. This PR does all warnings using nightly since they all looked like reasonable things to fix.

Needed for CI to pass in other open PRs.

Clippy emits a bunch of warnings of form:

 warning: casting to the same type is unnecessary ...

As suggested, remove the unnecessary casts.
Clippy emits:

 warning: deref which would be done by auto-deref

As suggested, remove the unnecessary deref (`*`).
Clippy emits:

 warning: binary comparison to literal `Option::None`

As suggested, use `find.is_none()` instead of comparison with `None`.
Clippy emits:

 warning: the borrowed expression implements the required traits

As suggested, remove the unnecessary explicit borrow.
Copy link
Collaborator

@Kixunil Kixunil left a comment

Choose a reason for hiding this comment

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

ACK 1050fe9

Nice cleanups!

@@ -284,7 +284,7 @@ impl Decodable for BlockTransactionsRequest {
// Since the number of indices ultimately represent transactions,
// we can limit the number of indices to the maximum number of
// transactions that would be allowed in a vector.
let byte_size = (nb_indexes as usize)
let byte_size = (nb_indexes)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, it didn't complain about useless parentheses?

Copy link
Member

@sanket1729 sanket1729 Nov 4, 2022

Choose a reason for hiding this comment

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

We should remove this later. Merging this PR now to get the CI rolling

Copy link
Member Author

Choose a reason for hiding this comment

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

bah, I must be blind. Will fix, thanks.

@Kixunil Kixunil added code quality Makes code easier to understand and less likely to lead to problems trivial Obvious, easy and quick to review (few lines or doc-only...) one ack PRs that have one ACK, so one more can progress them labels Nov 4, 2022
Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

ACK 1050fe9.

@sanket1729 sanket1729 merged commit 932aaaa into rust-bitcoin:master Nov 4, 2022
@tcharding tcharding deleted the 11-04-clippy branch November 7, 2022 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Makes code easier to understand and less likely to lead to problems one ack PRs that have one ACK, so one more can progress them trivial Obvious, easy and quick to review (few lines or doc-only...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants