Skip to content

chore: fix clippy#27

Merged
cfallin merged 1 commit intobytecodealliance:mainfrom
andreiltd:fix-clippy
Mar 4, 2026
Merged

chore: fix clippy#27
cfallin merged 1 commit intobytecodealliance:mainfrom
andreiltd:fix-clippy

Conversation

@andreiltd
Copy link
Member

Fix clippy warnings, most of the fixes were applied using clippy --fix.

Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

Thanks!

FWIW I've avoided Clippy in the past because many of its lints and "fixes" are over-aggressive and opinionated, and sometimes make code worse (and likewise we feel that way in Wasmtime)... but most of these are actually pretty reasonable. On the other hand I'm not a huge fan of the load-bearing && in the workqueue-insertion bits (if changed is logically separate from the if set.insert, push on queue action).

I guess there's a question of whether we do this once-off or add a lint in CI. Obviously we don't want to have to keep doing a manual chore. I still don't like the idea of "default Clippy" being a gatekeeper, but maybe we could enable some lints?

Here's our Wasmtime config in the root Cargo.toml, where we start with no lints and turn some reasonable ones on, then we run it in CI. Maybe we could add that in this PR as well to make it sticky? And keep the diff to what that stripped-down config from Wasmtime produces?

@andreiltd
Copy link
Member Author

Understood! I followed your suggestions and copied the wasmtime lints and adjusted the diff to only fix those. The reason I added this is that I usually run clippy before pushing PR and my changes were drowned in other warnings so it was hard to tell if my changes were reasons for warnings or they were preexisting.

But also feel free to just close this PR without merging 🙂

Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

Looks great, thanks! I still speak Rust with a 2015-edition accent so it's good to have tooling to modernize various bits :-)

@cfallin cfallin merged commit c813fa3 into bytecodealliance:main Mar 4, 2026
11 checks passed
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