-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat: add manual_is_variant_and lint
#11865
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
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Jarcho (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
|
☔ The latest upstream changes (presumably #11866) made this pull request unmergeable. Please resolve the merge conflicts. |
d839b45 to
60a3490
Compare
|
☔ The latest upstream changes (presumably #11977) made this pull request unmergeable. Please resolve the merge conflicts. |
81ca127 to
87ef71a
Compare
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.
Sorry for the long delay. The implementation is fine, just have some small nits about it.
Can you change the name to something like manual_is_variant_and. Otherwise we'll end up with a pile of lints that do basically the same thing.
No worries! Thank you for your review Jarcho! I have tried to resolved the problems you raised. Can you please see if the new code looks good? |
map_unwrap_or_default lintmanual_is_variant_and lint
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.
Things look good. Just need to squash down the commits and then it's good.
eb6190b to
0c95e6c
Compare
All done! Have a good holiday! |
|
☔ The latest upstream changes (presumably #12004) made this pull request unmergeable. Please resolve the merge conflicts. |
0c95e6c to
c4a80f2
Compare
|
Thank you. @bors r+ |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
changelog: add a new lint [
manual_is_variant_and].option.map(f).unwrap_or_default()andresult.map(f).unwrap_or_default()withoption.is_some_and(f)andresult.is_ok_and(f)wherefis a function or closure that returnsbool.is_some_andandis_ok_andwas stabilisedFor example, for the following code:
It suggests to instead write: