-
Notifications
You must be signed in to change notification settings - Fork 186
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
CI: Run Clippy for (nearly) all targets. #447
Conversation
2bfdbca
to
f40ba84
Compare
Ensure that Clippy is looking at as much of the target-specific code as practical.
@@ -25,7 +25,7 @@ const TRUE: BOOLEAN = 1u8; | |||
|
|||
pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> { | |||
// Prevent overflow of u32 | |||
for chunk in dest.chunks_mut(u32::max_value() as usize) { | |||
for chunk in dest.chunks_mut(u32::MAX as usize) { |
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 the changes to tests.yml, clippy now sees this code.
The CI failure is the typical Windows wasm32 error. |
@@ -65,6 +66,14 @@ jobs: | |||
- run: cargo test --features=custom # custom should do nothing here | |||
- if: ${{ matrix.toolchain == 'nightly' }} | |||
run: cargo test --benches | |||
- if: ${{ matrix.toolchain == 'stable' }} | |||
run: cargo clippy --all |
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.
Running Clippy on stable would mean that we can get failing CI on new stable releases. Usually I prefer to fix Clippy version and bump it manually form time to time.
Maybe we should add a single Clippy job which will check a set of targets using cargo clippy --target
?
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.
+1 to using some fixed nightly Clippy and running it on a bunch of targets in a single job
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.
Why Nightly?
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.
If we want to run clippy on targets which don't have libstd pre built, we will have to use -Z build_std
Closing in favor of #477. |
Ensure that Clippy is looking at as much of the target-specific code as practical.