Skip to content

Fix map_unwrap_or fail to cover Result::unwrap_or #15718

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.

Sign up for GitHub

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

Jump to bottom
Open
profetia wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix map_unwrap_or fail to cover Result::unwrap_or #15718

profetia wants to merge 3 commits into from
+445 −243

Conversation

@profetia
Copy link
Contributor

@profetia profetia commented Sep 19, 2025 •
edited by rustbot
Loading

Closes #15713
Closes #15714
Closes #15752
Closes #16258

changelog: [map_unwrap_or] add cover for Result::unwrap_or

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 19, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 19, 2025

r? @blyxyas

rustbot has assigned @blyxyas.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@github-actions
Copy link

github-actions bot commented Sep 19, 2025 •
edited
Loading

Lintcheck changes for a0f09e9

Lint Added Removed Changed
clippy::map_unwrap_or 15 0 5

This comment will be updated if you push new changes

@samueltardieu
Copy link
Member

samueltardieu commented Sep 19, 2025

Unless you have a good reason not to, please use map_unwrap_or_fixable.rs for the tests.

@profetia
Copy link
Contributor Author

profetia commented Sep 19, 2025

Moved. Thank you!

@rustbot

This comment has been minimized.

Sign in to view
@profetia
Copy link
Contributor Author

profetia commented Sep 23, 2025

Also fixes #15713

@profetia
Copy link
Contributor Author

profetia commented Sep 24, 2025

Also closes #15752

samueltardieu
samueltardieu requested changes Sep 24, 2025
View reviewed changes
Copy link
Member

@samueltardieu samueltardieu left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

If possible, you should squash together the two commits "Apply map_unwrap to Clippy itself" and make it the first commit). This way, after every commit, Clippy tests still pass (including dogfood), and it makes it easy to bisect should something go wrong. Then each "fix" commit can be put on top.

View changes since this review

@profetia
Copy link
Contributor Author

profetia commented Sep 29, 2025

Updated. Thank you

ada4a
ada4a reviewed Oct 1, 2025
View reviewed changes
@rustbot

This comment has been minimized.

Sign in to view
@rustbot

This comment has been minimized.

Sign in to view
@profetia
Copy link
Contributor Author

profetia commented Oct 21, 2025

r? clippy

@rustbot rustbot assigned samueltardieu and unassigned blyxyas Oct 21, 2025
samueltardieu
samueltardieu requested changes Oct 23, 2025
View reviewed changes
Copy link
Member

@samueltardieu samueltardieu left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

Could you also squash the commits please? The current separation makes it harder to read and review.

View changes since this review

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Oct 23, 2025
@profetia profetia force-pushed the issue15714 branch 2 times, most recently from 0a176fc to 081f15f Compare October 23, 2025 21:32
@profetia
Copy link
Contributor Author

profetia commented Oct 23, 2025

Done. Thank you!

samueltardieu
samueltardieu requested changes Oct 23, 2025
View reviewed changes
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Oct 23, 2025
@profetia
Copy link
Contributor Author

profetia commented Nov 5, 2025

r? clippy

@rustbot rustbot assigned dswij and unassigned samueltardieu Nov 5, 2025
@profetia
Copy link
Contributor Author

profetia commented Nov 25, 2025

r? clippy

@rustbot rustbot assigned Jarcho and unassigned dswij Nov 25, 2025
@profetia
Copy link
Contributor Author

profetia commented Dec 7, 2025

r? clippy

@rustbot rustbot assigned Centri3 and unassigned Jarcho Dec 7, 2025
Centri3
Centri3 reviewed Dec 17, 2025
View reviewed changes
@rustbot

This comment has been minimized.

Sign in to view
@rustbot

This comment has been minimized.

Sign in to view
@rustbot
Copy link
Collaborator

rustbot commented Dec 18, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@profetia profetia requested a review from Centri3 December 18, 2025 18:24
@profetia profetia mentioned this pull request Dec 18, 2025
Open
@profetia
Copy link
Contributor Author

profetia commented Dec 18, 2025

Also closes #16258

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

Reviewers

@samueltardieu samueltardieu Awaiting requested review from samueltardieu

@Centri3 Centri3 Awaiting requested review from Centri3

1 more reviewer

@ada4a ada4a ada4a left review comments

Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees

@Centri3 Centri3

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Milestone

No milestone

8 participants

@profetia @rustbot @samueltardieu @ada4a @Centri3 @Jarcho @dswij @blyxyas