Conversation
|
To test how this works when a clippy lint is triggered, I tried creating a dummy PR here that should trigger a clippy warning. |
joshlf
left a comment
There was a problem hiding this comment.
Thanks for this! Can you move the check to be one of the steps under the existing "Build & Test" matrix? Specifically, I'd suggest adding it just after the existing "Check" step - it should be basically a carbon copy of that step. It'd be good to make sure we get coverage on all toolchain/target combinations.
|
Sure! I didn't realize that clippy warning could differ based on toolchain/target. |
|
Yeah, any code that is conditionally compiled, and possibly even code that isn't but whose behavior differs by platform (e.g., some clippy lints (like this one) make use of the word size on the platform). |
e567f2c to
2fdcaab
Compare
|
I think you can fix the current CI failures by invoking Clippy directly from the command line. It comes default with Rust installations now, so you don't need a separate action for it. All I did here was copy-paste the "Check" action and edit it to invoke |
|
Unfortunately, I didn't have much time to experiment with this. Surely your solution would be easier. I thought the GitHub Action could be more convenient since it can also show inline error messages like this: Would that be desirable for you? I think I'll have more time to work on this next week. If instead it's a priority for you, feel free to close this PR! |
|
Ah that looks really nice! Unfortunately, it looks like it won't work for people submitting PRs from forked repositories (which is basically everyone other than me). Also, since we're enforcing this (rather than just using Clippy lints as suggestions), hopefully most PRs won't have many if any lints, and so a nice UI isn't that important. I opened #69 to track making it easier to run all of our tests (including Clippy) locally. Given those considerations, I think #51 (comment) makes the most sense. |
|
Oh, I see! I didn't understand this limitation existed.. Trying now the changes you suggested. |
e70eeb4 to
99c24a6
Compare
|
So, it seems like some clippy errors are triggered when the "alloc" features is enabled! Can reproduce locally with: |
|
Ooops! I think I fixed the rest in #71 . |
Add a CI job to run the clippy linting software. Fixes google#49
|
Rebased and pushed. Looks good now! |
Add a CI test to run the Clippy linter. Fixes #49

Add a CI job to run the clippy linting software.
Fixes #49