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.

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

Add linter for a single element for loop #6109

Merged
merged 2 commits into from
Oct 25, 2020
Merged

Add linter for a single element for loop #6109

merged 2 commits into from
Oct 25, 2020

Conversation

patrickelectric
Copy link
Contributor

@patrickelectric patrickelectric commented Oct 3, 2020 •
edited
Loading

changelog: Fixes #1540, check for vectors that contain a single element in a for loop

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @flip1995 (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 3, 2020
@patrickelectric patrickelectric force-pushed the single_element_for_check branch 4 times, most recently from e8307d3 to 32cab51 Compare October 3, 2020 20:07
giraffate
giraffate reviewed Oct 4, 2020
View reviewed changes
clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
@patrickelectric patrickelectric force-pushed the single_element_for_check branch 3 times, most recently from cb2be56 to 7babff8 Compare October 4, 2020 16:33
@patrickelectric
Copy link
Contributor Author

ping @flip1995

1 similar comment
@patrickelectric
Copy link
Contributor Author

ping @flip1995

@flip1995
Copy link
Member

Sorry for taking so long. I currently have 15 PRs I'm actively reviewing/triaging besides doing other organizational stuff for Clippy. I usually review on weekends, so pinging doesn't really help to speed up my reviews.

@bors
Copy link
Contributor

bors commented Oct 10, 2020

☔ The latest upstream changes (presumably #5727) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@flip1995 flip1995 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 10, 2020
flip1995
flip1995 reviewed Oct 10, 2020
View reviewed changes
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution and welcome to Clippy!

tests/ui/single_element_loop.rs Show resolved Hide resolved
clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
@patrickelectric patrickelectric force-pushed the single_element_for_check branch 5 times, most recently from 58a0cbc to 1a1f347 Compare October 10, 2020 22:02
@patrickelectric
Copy link
Contributor Author

Hi @flip1995, sorry for the ping, is just something that I do after a couple of days if a PR does not get a review, but since you mainly review PRs during the weekends I'll stop ping here. Sorry for the bothering!
I just fixed the requested changes and rebased over the current master branch, let me know if anything else is necessary.
Have a great weekend!

flip1995
flip1995 requested changes Oct 17, 2020
View reviewed changes
clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
tests/ui/single_element_loop.rs Show resolved Hide resolved
@patrickelectric patrickelectric force-pushed the single_element_for_check branch 4 times, most recently from 220a4a6 to 094e0f2 Compare October 17, 2020 21:59
@patrickelectric patrickelectric force-pushed the single_element_for_check branch from 094e0f2 to 51c7834 Compare October 18, 2020 00:35
@patrickelectric patrickelectric force-pushed the single_element_for_check branch from 51c7834 to de937ab Compare October 19, 2020 12:32
@patrickelectric
Add linter for a single element for loop
ec23db9 
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
@patrickelectric
tests: if_same_then_else2: Ignore single_element_loop lint
ba1ca19 
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
@patrickelectric patrickelectric force-pushed the single_element_for_check branch from de937ab to ba1ca19 Compare October 19, 2020 12:53
@patrickelectric
Copy link
Contributor Author

ping @flip1995

@flip1995
Copy link
Member

@bors r+

Thanks!

Sorry for taking so long. I'll start a new job in a new town next month and have little time for reviews currently. I hope it settles down soon and I have more time and energy for Clippy again.

@bors
Copy link
Contributor

bors commented Oct 25, 2020

📌 Commit ba1ca19 has been approved by flip1995

@bors
Copy link
Contributor

bors commented Oct 25, 2020

⌛ Testing commit ba1ca19 with merge e0e617a...

@bors
Copy link
Contributor

bors commented Oct 25, 2020

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing e0e617a to master...

@bors bors merged commit e0e617a into rust-lang:master Oct 25, 2020
This was referenced Oct 25, 2020
Closed
Closed
Merged
Merged
@patrickelectric
Copy link
Contributor Author

Hi @flip1995, thank you for the review, I was just a bit worried about the hacktoberfest schedule.
Congratulations for the new job and I hope all is well in the coming weeks!

@flip1995
Copy link
Member

IIUC for hacktoberfest only the opening date of the PR is relevant. 😉

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

@giraffate giraffate giraffate left review comments

@flip1995 flip1995 flip1995 approved these changes

Assignees

@flip1995 flip1995

Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Lint for iterating over a slice with one (or zero) element
5 participants
@patrickelectric @rust-highfive @flip1995 @bors @giraffate