-
Notifications
You must be signed in to change notification settings - Fork 324
chore(ci): enforce clippy lints in CI #814
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
base: master
Are you sure you want to change the base?
Conversation
| components: clippy | ||
| - uses: actions-rs-plus/clippy-check@v2 | ||
| with: | ||
| args: --all --all-features --all-targets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the set of clippy lints that have been enabled?
I think we want to pin this to a specific rustc to avoid random breakages on each new Rust version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with this config, the lints are clippy::all.
happy to pin rust to a specific version, the problem is remembering to bump this regularly. Renovate can do this, dependabot cant. not sure if the maintainers would be amenable to adding renovate just for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah i see there's already a pinned nightly and it's wildly out of date...
might look at addressing that separately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see #815
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not too worried about bumping the bytes crate clippy lint version. And clippy::all is a really really broad category.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you have a particular lint set in mind?
i think concerns about the broadness of the category are obviated by pinning the toolchain. any warnings you're not happy with can be selectively disabled, and new ones only creep in predictably on bumping the pinned version.
this PR is stacked on #813
it addresses the last couple of clippy warnings and adds a CI check