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

A few clippy-suggested lints #342

Merged
merged 6 commits into from
Nov 13, 2023
Merged

A few clippy-suggested lints #342

merged 6 commits into from
Nov 13, 2023

Conversation

nyurik
Copy link
Contributor

@nyurik nyurik commented Nov 10, 2023

Most of these seemed reasonable, but feel free to push back on any of them. Clippy also recommended removing a few unused &self, but maybe will do that in a separate PR

@nyurik
A few clippy-suggested lints
1413166 
Most of these seemed reasonable, but feel free to push back on any of them. Clippy also recommended removing a few unused `&self`, but maybe will do that in a separate PR
@tatsuya6502 tatsuya6502 self-requested a review November 10, 2023 13:26
@tatsuya6502 tatsuya6502 added this to the v0.12.2 milestone Nov 10, 2023
tatsuya6502
tatsuya6502 reviewed Nov 13, 2023
View reviewed changes
Copy link
Member

@tatsuya6502 tatsuya6502 left a comment

Choose a reason for hiding this comment

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

Thank you for working on this PR. I added suggestions for reverting some comment changes and adding a comment back that was deleted by accident.

It was nice to know that Clippy can detect unnecessary async fn. What is the name for the lint?

src/cht/segment.rs Outdated Show resolved Hide resolved
src/cht/segment.rs Outdated Show resolved Hide resolved
src/cht/segment.rs Outdated Show resolved Hide resolved
src/cht/segment.rs Outdated Show resolved Hide resolved
src/common/timer_wheel.rs Show resolved Hide resolved
src/sync/builder.rs Outdated Show resolved Hide resolved
src/sync/builder.rs Outdated Show resolved Hide resolved
src/sync/cache.rs Outdated Show resolved Hide resolved
src/sync/cache.rs Outdated Show resolved Hide resolved
src/sync/cache.rs Outdated Show resolved Hide resolved
nyurik and others added 5 commits November 13, 2023 12:41
@nyurik @tatsuya6502
Update src/cht/segment.rs
83dc2c9 
Co-authored-by: Tatsuya Kawano <tatsuya@hibaridb.org>
@nyurik @tatsuya6502
Update src/cht/segment.rs
0a54847 
Co-authored-by: Tatsuya Kawano <tatsuya@hibaridb.org>
@nyurik @tatsuya6502
Update src/cht/segment.rs
b90a139 
Co-authored-by: Tatsuya Kawano <tatsuya@hibaridb.org>
@nyurik @tatsuya6502
Update src/cht/segment.rs
15c7a7d 
Co-authored-by: Tatsuya Kawano <tatsuya@hibaridb.org>
@nyurik @tatsuya6502
Apply suggestions from code review
4e601c9 
Co-authored-by: Tatsuya Kawano <tatsuya@hibaridb.org>
@nyurik
Copy link
Contributor Author

nyurik commented Nov 13, 2023

Thanks @tatsuya6502 for such detailed review! The lint name is clippy::unused_async. Let me know if anything else needs to be changed.

tatsuya6502
tatsuya6502 approved these changes Nov 13, 2023
View reviewed changes
Copy link
Member

@tatsuya6502 tatsuya6502 left a comment

Choose a reason for hiding this comment

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

Thank you for updating. LGTM. Merging into the main branch.

@tatsuya6502 tatsuya6502 merged commit 54c4f84 into moka-rs:main Nov 13, 2023
16 checks passed
@nyurik nyurik deleted the linting branch November 13, 2023 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tatsuya6502 tatsuya6502 tatsuya6502 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.12.2
Development

Successfully merging this pull request may close these issues.
2 participants
@nyurik @tatsuya6502