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

Refactor unit types #6889

Merged
merged 6 commits into from
Mar 16, 2021
Merged

Refactor unit types #6889

merged 6 commits into from
Mar 16, 2021

Conversation

Y-Nak
Copy link
Contributor

@Y-Nak Y-Nak commented Mar 12, 2021 •
edited
Loading

Ref: #6724
r? @flip1995

Changes:

  1. Extract unit_types from types group.
  2. Move lints of unit_types to their own modules.

Notes:
Other lints of unit_types is still scattered around the clippy_lints, e.g. result_unit_err or option_map_unit_fn.
These should be addressed in another PR.

changelog: none

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 12, 2021
@Y-Nak Y-Nak mentioned this pull request Mar 12, 2021
Closed
4 tasks
flip1995
flip1995 approved these changes Mar 15, 2021
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.

This is especially a great refactor, since it combines 3 lint passes into 1. Only a NIT, that can be fixed in this PR, I think.

clippy_lints/src/unit_types/utils.rs Outdated Show resolved Hide resolved
@flip1995
Copy link
Member

@bors delegate+ p=1

You can approve the PR yourself so it doesn't come to merge conflicts, once you've addressed my comment above.

@bors
Copy link
Contributor

bors commented Mar 15, 2021

✌️ @Y-Nak can now approve this pull request

flip1995
flip1995 reviewed Mar 15, 2021
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.

Those should be all the places this function is used.

clippy_lints/src/unit_types/unit_cmp.rs Outdated Show resolved Hide resolved
clippy_lints/src/unit_types/unit_cmp.rs Outdated Show resolved Hide resolved
clippy_lints/src/unit_types/unit_arg.rs Outdated Show resolved Hide resolved
clippy_lints/src/unit_types/let_unit_value.rs Outdated Show resolved Hide resolved
@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 Mar 15, 2021
@bors
Copy link
Contributor

bors commented Mar 15, 2021

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

@Y-Nak Y-Nak force-pushed the refactor-unit-types branch from ecd25f7 to 2909b1c Compare March 16, 2021 01:37
@Y-Nak
Copy link
Contributor Author

Y-Nak commented Mar 16, 2021 •
edited
Loading

Thanks for your review!
@bors r+

@bors
Copy link
Contributor

bors commented Mar 16, 2021

📌 Commit 2909b1c has been approved by Y-Nak

@bors
Copy link
Contributor

bors commented Mar 16, 2021

⌛ Testing commit 2909b1c with merge 59cf93a...

bors added a commit that referenced this pull request Mar 16, 2021
@bors
Auto merge of #6889 - Y-Nak:refactor-unit-types, r=Y-Nak
59cf93a 
Refactor unit types

Ref:  #6724
r? `@flip1995`

Changes:
1. Extract `unit_types` from `types` group.
2. Move lints of `unit_types` to their own modules.

Notes:
Other lints of `unit_types` is still scattered around the `clippy_lints`, e.g. `result_unit_err` or `option_map_unit_fn`.
These should be addressed in another PR.

changelog: none
@Y-Nak
Copy link
Contributor Author

Y-Nak commented Mar 16, 2021

@bors r-

@Y-Nak
Copy link
Contributor Author

Y-Nak commented Mar 16, 2021

Oops! I thought delegate+ automatically adds flip1995 as a reviewer.

@bors
Copy link
Contributor

bors commented Mar 16, 2021

☀️ Try build successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Build commit: 59cf93a (59cf93a8f5926cd803f0656dee13f8fffa9b1946)

@Y-Nak
Copy link
Contributor Author

Y-Nak commented Mar 16, 2021

@bors r=flip1995

@bors
Copy link
Contributor

bors commented Mar 16, 2021

📌 Commit 2909b1c has been approved by flip1995

@bors
Copy link
Contributor

bors commented Mar 16, 2021

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

  1. git checkout refactor-unit-types (switch to your branch)
  2. git fetch upstream master (retrieve the latest master)
  3. git rebase upstream/master -p (rebase on top of it)
  4. Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
  5. git push self refactor-unit-types --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message 
Auto-merging clippy_lints/src/types/mod.rs
CONFLICT (content): Merge conflict in clippy_lints/src/types/mod.rs
Automatic merge failed; fix conflicts and then commit the result.

@bors
Copy link
Contributor

bors commented Mar 16, 2021

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

@Y-Nak Y-Nak force-pushed the refactor-unit-types branch from 2909b1c to 5a439f5 Compare March 16, 2021 02:30
@Y-Nak
Copy link
Contributor Author

Y-Nak commented Mar 16, 2021

@bors r=flip1995 p=1

@bors
Copy link
Contributor

bors commented Mar 16, 2021

📌 Commit 5a439f5 has been approved by flip1995

@bors
Copy link
Contributor

bors commented Mar 16, 2021

⌛ Testing commit 5a439f5 with merge 81c0154...

bors added a commit that referenced this pull request Mar 16, 2021
@bors
Auto merge of #6889 - Y-Nak:refactor-unit-types, r=flip1995
81c0154 
Refactor unit types

Ref:  #6724
r? `@flip1995`

Changes:
1. Extract `unit_types` from `types` group.
2. Move lints of `unit_types` to their own modules.

Notes:
Other lints of `unit_types` is still scattered around the `clippy_lints`, e.g. `result_unit_err` or `option_map_unit_fn`.
These should be addressed in another PR.

changelog: none
@bors
Copy link
Contributor

bors commented Mar 16, 2021

💔 Test failed - checks-action_test

@Y-Nak
Copy link
Contributor Author

Y-Nak commented Mar 16, 2021

@bors retry

@bors
Copy link
Contributor

bors commented Mar 16, 2021

⌛ Testing commit 5a439f5 with merge c701c30...

@bors
Copy link
Contributor

bors commented Mar 16, 2021

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

@bors bors merged commit c701c30 into rust-lang:master Mar 16, 2021
@Y-Nak Y-Nak deleted the refactor-unit-types branch March 16, 2021 03:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@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.
4 participants
@Y-Nak @flip1995 @bors @rust-highfive