-
Notifications
You must be signed in to change notification settings - Fork 22
Add clippy to CI (as non blocking warnings) #19
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
Conversation
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.
Sorry for landing this!
.github/workflows/ci.yml
Outdated
| # For now, we only check `fearless_simd_gen`, since `fearless_simd` uses `prettyplease`. | ||
| - name: cargo fmt | ||
| run: cd fearless_simd_gen && cargo fmt --check | ||
| run: cargo fmt --check |
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 we need to pass --all here or does it work just like that?
|
|
||
| [workspace.lints] | ||
|
|
||
| # LINEBENDER LINT SET - Cargo.toml - v5 |
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 sure whether we really want all of those lints as some might become a bit annoying to deal with and unnecessary (see also currently failing CI) since we are dealing with code generation, but I'm okay with leaving it as is for now.
Context
This PR adds the Linebender clippy warnings to CI. Because CI doesn't block us it's ok to land. Ideally I'd like all the clippy lints to become errors and for us to burn down the clippy warnings.
I also deleted some embarrassing
println!debug statements that I accidentally merged intomain🤦Finally, I increased the scope of the
fmtcheck to all packages.Goal
In the longer term I think it's important we conform to clippy, if only because it'll catch issues.
As a random note, I couldn't use
fearless_simdwithvellousingbytemuckversion1.23.1, due to conflicting dependencies invello. Using1.23.0matches the version of bytemuck in vello.