Skip to content
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

Use if-let guards in the codebase and various other pattern cleanups #88066

Merged
merged 2 commits into from
Aug 26, 2021

Conversation

LeSeulArtichaut
Copy link
Contributor

Dogfooding if-let guards as experimentation for the feature.

Tracking issue #51114. Conflicts with #87937.

@rust-highfive
Copy link
Collaborator

r? @nagisa

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 15, 2021
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@nagisa
Copy link
Member

nagisa commented Aug 15, 2021

Please split up the different kinds of cleanups (i.e. those that use if-let-guards and those that don't) into different commits so that the changes are better isolated and easier to review.

I think the scope of this PR is otherwise reasonable enough for dogfooding purposes.

@LeSeulArtichaut LeSeulArtichaut force-pushed the patterns-cleanups branch 2 times, most recently from f01bac0 to e8f3821 Compare August 16, 2021 15:34
@LeSeulArtichaut
Copy link
Contributor Author

@nagisa I split the changes in two commits as you requested. Since the bulk of the changes is actually reindenting the code (caused by removing an if-let block), the diff will look less noisy if viewed with whitespace changes hidden.

I also forgot to mention that rustfmt doesn't yet support formatting if-let guards (rust-lang/rustfmt#4955) so I tried my best to format them by hand.

@nagisa
Copy link
Member

nagisa commented Aug 23, 2021

Seems nice. Up to you if you want to handle the comments above.

@bors delegate=LeSeulArtichaut

@bors
Copy link
Contributor

bors commented Aug 23, 2021

✌️ @LeSeulArtichaut can now approve this pull request

@LeSeulArtichaut
Copy link
Contributor Author

@nagisa Replaced the problematic patterns with if-let guards.

@LeSeulArtichaut
Copy link
Contributor Author

@bors r=nagisa

@bors
Copy link
Contributor

bors commented Aug 23, 2021

📌 Commit f83887d5dc94c25acfbdbc2b51aa19bd7f5ff1ee has been approved by nagisa

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 23, 2021
@LeSeulArtichaut
Copy link
Contributor Author

Removed all #[allow(incomplete_features)] due to the use of if-let guards.
@bors r=nagisa

@bors
Copy link
Contributor

bors commented Aug 25, 2021

📌 Commit 1cfb326b5caba719be0553c1b0bb56c974955dbc has been approved by nagisa

@rust-log-analyzer

This comment has been minimized.

@LeSeulArtichaut LeSeulArtichaut force-pushed the patterns-cleanups branch 2 times, most recently from e4d4aa3 to 2b0c8ff Compare August 25, 2021 18:23
@LeSeulArtichaut
Copy link
Contributor Author

@bors r=nagisa

@bors
Copy link
Contributor

bors commented Aug 25, 2021

📌 Commit 2b0c8ff has been approved by nagisa

@bors
Copy link
Contributor

bors commented Aug 26, 2021

⌛ Testing commit 2b0c8ff with merge 76e755c...

@bors
Copy link
Contributor

bors commented Aug 26, 2021

☀️ Test successful - checks-actions
Approved by: nagisa
Pushing 76e755c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 26, 2021
@bors bors merged commit 76e755c into rust-lang:master Aug 26, 2021
@rustbot rustbot added this to the 1.56.0 milestone Aug 26, 2021
@LeSeulArtichaut LeSeulArtichaut deleted the patterns-cleanups branch August 26, 2021 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants