Skip to content

Fix clippy lints in verify#265

Closed
0xZaddyy wants to merge 1 commit intorust-bitcoin:masterfrom
0xZaddyy:lint--verify-warnings
Closed

Fix clippy lints in verify#265
0xZaddyy wants to merge 1 commit intorust-bitcoin:masterfrom
0xZaddyy:lint--verify-warnings

Conversation

@0xZaddyy
Copy link
Contributor

This PR fixes all Clippy lints in the verify crate by applying idiomatic Rust improvements and minor cleanups.

@tcharding
Copy link
Member

Why do you keep closing PRs and re-opening new ones fella? You can just force push to the existing one. I'll review this one now though.

Copy link
Collaborator

@jamillambert jamillambert left a comment

Choose a reason for hiding this comment

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

ACK d9b50c5

Can you write a more descriptive commit message next time? Have a look at How to Write a Git Commit Message for advice.
Thanks.

@0xZaddyy
Copy link
Contributor Author

Alright boss, thanks!

Some(Return::Type(_)) => true,
_ => false,
};
let requires = matches!(method.ret, Some(Return::Type(_)));
Copy link
Member

Choose a reason for hiding this comment

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

Can you make this a separate patch please. Its a bit more work but good practice in writing commit logs and explaining your changes. Gives us reviewers confidence you know what you are doing and not just following the compiler blindly.

Thanks

else if model::type_exists(version, method_name)? {
eprintln!("found model type when none expected: {}", output_method(out));
}

Copy link
Member

Choose a reason for hiding this comment

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

I don't personally like this lint. The first block of changes are ok-ish. The second block are definitely worse. I'd prefer to just allow the lint.

@tcharding
Copy link
Member

Hey mate, this one was fiddly enough that I decided to just do it. Thanks for you efforts though.

Please see #246 if you are interested in how I went about it.

@tcharding tcharding closed this Jun 24, 2025
@0xZaddyy
Copy link
Contributor Author

Alright, thank you sir.
Taking notes of everything, I'll improve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants