-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Extend if_then_some_else_none
to also suggest bool::then_some
#9289
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @xFrednet (or someone else) soon. Please see the contribution instructions for more information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. As a suggestion, could you mention bool:then_some
in the lint description? The current example suggests bool::then
which could/should be bool::then_some
. As another change, you could just mention bool::then_some
in the "Why is this bad?" section. Something like:
For simple calculation and known values you can also use
bool::then_some
which is eagerly avaluated in comparison tobool:then
.
Otherwise, this is ready to merge. 👍 Thank you for the contribution and patience 🙃
Thanks for the review! I've pushed some changes to the lint's description. |
Perfect, thank you for the implementation and quick follow up. I hope you had fun while working on Clippy 🙃 @bors r+ |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Closes #9094.
changelog: Extend
if_then_some_else_none
to also suggestbool::then_some