Skip to content

Conversation

@danieleades
Copy link

this PR is stacked on #813

it addresses the last couple of clippy warnings and adds a CI check

components: clippy
- uses: actions-rs-plus/clippy-check@v2
with:
args: --all --all-features --all-targets
Copy link
Contributor

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.

Copy link
Author

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?

Copy link
Author

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

Copy link
Author

Choose a reason for hiding this comment

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

see #815

Copy link
Contributor

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.

Copy link
Author

@danieleades danieleades Jan 5, 2026

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.

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.

2 participants