Skip to content

clippy: fix manual_let_else lint#690

Merged
Amanieu merged 1 commit intorust-lang:masterfrom
xtqqczze:clippy/manual_let_else
Mar 4, 2026
Merged

clippy: fix manual_let_else lint#690
Amanieu merged 1 commit intorust-lang:masterfrom
xtqqczze:clippy/manual_let_else

Conversation

@xtqqczze
Copy link
Contributor

@clarfonthey
Copy link
Contributor

Why are you separating all of these into separate PRs? Also, what justification do you have for fixing these in this project when they're not clippy-default lints, since that means they're not recommended for general use, and mostly just preference?

@xtqqczze
Copy link
Contributor Author

I prefer to keep PRs small and not mix unrelated changes, it’s usually easier to review too. It avoids conflating independent discussions, and if one change needs revision, it doesn’t block the others.

As for the pedantic lints, they can still catch patterns that affect readability or maintainability. In the case of manual_let_else, I think the changes are an improvement.

@clarfonthey
Copy link
Contributor

That makes sense, although they are non-default for a reason.

In this case, it actually looks like it might make sense to use the unwrap_unchecked methods for a bunch of these since our MSRV supports it. Don't think there's currently a clippy lint checking for that.

@xtqqczze
Copy link
Contributor Author

xtqqczze commented Feb 21, 2026

Good idea! I’ve opened #693 since the changes turned out to be a bit more involved. I’ll mark this PR as draft for now due to conflicts.

@xtqqczze xtqqczze marked this pull request as draft February 21, 2026 03:53
@xtqqczze xtqqczze force-pushed the clippy/manual_let_else branch from 92ea47d to 3027ac0 Compare February 27, 2026 20:45
@xtqqczze xtqqczze marked this pull request as ready for review February 27, 2026 20:45
@clarfonthey
Copy link
Contributor

Looking at the new version, I guess this is okay to merge. I'm kind of indifferent on this style and don't think that let-else is strictly an improvement, and there's a reason why clippy doesn't make this lint default. Will let someone else merge if they think it's an improvement.

@xtqqczze xtqqczze force-pushed the clippy/manual_let_else branch from 3027ac0 to 3f15a02 Compare February 27, 2026 22:11
@xtqqczze
Copy link
Contributor Author

Why are you separating all of these into separate PRs?

Yeah, separate PRs aren't ideal when they create merge conflicts in Cargo.toml...

@Amanieu Amanieu added this pull request to the merge queue Mar 4, 2026
Merged via the queue into rust-lang:master with commit cc408e7 Mar 4, 2026
25 checks passed
@xtqqczze xtqqczze deleted the clippy/manual_let_else branch March 4, 2026 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants