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

chore: run clippy against all targets #546

Merged
merged 13 commits into from
Apr 22, 2024

Conversation

Rustin170506
Copy link
Collaborator

@Rustin170506 Rustin170506 commented Apr 15, 2024

This would help us to check if there is any clippy warning in the tests.

I found it after #544. There are some unused imports in our test code.

Use cargo clippy --workspace --all-targets --no-deps -- -D warnings to test all targes which include the test targets.

@Rustin170506 Rustin170506 requested a review from a team as a code owner April 15, 2024 11:59
@Rustin170506 Rustin170506 force-pushed the rustin-patch-clippy-all branch 3 times, most recently from f818034 to 77c0539 Compare April 15, 2024 12:15
@Rustin170506 Rustin170506 requested a review from hds April 15, 2024 12:19
Copy link
Collaborator

@hds hds left a comment

Choose a reason for hiding this comment

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

Good idea! Would be good to avoid the unrelated formatting changes in the GitHub Actions file though.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

overall, this looks good, thanks for the lint fixes! i agree with @hds that we should undo the YAML formatting changes before merging.

console-subscriber/tests/spawn.rs Show resolved Hide resolved
console-subscriber/tests/support/task.rs Outdated Show resolved Hide resolved
@Rustin170506 Rustin170506 force-pushed the rustin-patch-clippy-all branch 2 times, most recently from 670d55a to 4c6daa4 Compare April 17, 2024 11:50
Copy link
Collaborator Author

@Rustin170506 Rustin170506 left a comment

Choose a reason for hiding this comment

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

🔢 Self-check (PR reviewed by myself and ready for feedback.)

@Rustin170506 Rustin170506 requested review from hawkw and hds April 17, 2024 12:02
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

looks good to me, thank you!

@Rustin170506 Rustin170506 enabled auto-merge (squash) April 18, 2024 00:25
@Rustin170506
Copy link
Collaborator Author

@hds This PR needs your approval before I can merge it. Could you please take a look? Thanks!
image

Copy link
Collaborator

@hds hds 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 adding this, great idea!

@Rustin170506 Rustin170506 merged commit 3193fde into tokio-rs:main Apr 22, 2024
17 checks passed
@hds
Copy link
Collaborator

hds commented Apr 22, 2024

@hds This PR needs your approval before I can merge it. Could you please take a look? Thanks!

image

Sorry! I didn't realize I was blocking this one. 🤦‍♀️

@Rustin170506
Copy link
Collaborator Author

Sorry! I didn't realize I was blocking this one. 🤦‍♀️

No worry! Thank you ❤️

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