Skip to content
This repository has been archived by the owner on Apr 18, 2019. It is now read-only.

Correctly run clippy in CI #141

Merged
merged 1 commit into from
Mar 20, 2019
Merged

Correctly run clippy in CI #141

merged 1 commit into from
Mar 20, 2019

Conversation

baumanj
Copy link
Contributor

@baumanj baumanj commented Mar 19, 2019

Clippy leverages cargo check, so running it after other tests may cause some lints to not be triggered. To increase effectiveness (and move fatal errors toward the bottom of the CI output), run clippy first.

Clippy leverages `cargo check`, so running it after other tests may
cause some lints to not be triggered. To increase effectiveness (and
move fatal errors toward the bottom of the CI output), run clippy first.

Signed-off-by: Jon Bauman <[email protected]>
@baumanj baumanj self-assigned this Mar 19, 2019
@thesentinels
Copy link
Contributor

Thanks for the pull request! Here is what will happen next:

  1. Your PR will be reviewed by the maintainers
  2. If everything looks good, one of them will approve it, and your PR will be merged.

Thank you for contributing!

Copy link
Contributor

@christophermaier christophermaier left a comment

Choose a reason for hiding this comment

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

Looks good, but can you elaborate a bit on the following?

so running it after other tests may cause some lints to not be triggered.

@raskchanky
Copy link
Contributor

@christophermaier I'm guessing the issue is that running cargo check after doing cargo build or cargo test won't actually build anything, since cargo is smart enough to detect that no code was changed.

@raskchanky raskchanky merged commit 48b2708 into master Mar 20, 2019
@raskchanky raskchanky deleted the core-run-clippy-in-ci branch March 20, 2019 17:09
@christophermaier
Copy link
Contributor

Hrmm... interesting 🤔

Does that suggest that we'd need to run cargo clean before running clippy? That seems a tad extreme.

@baumanj
Copy link
Contributor Author

baumanj commented Mar 21, 2019

Does that suggest that we'd need to run cargo clean before running clippy? That seems a tad extreme.

In this case, I think it suffices to move it to the first position so that we don't have any previous artifacts, but it is indeed possible for clippy to miss things on account of the erroneous code not being reconsidered at all.

See rust-lang/rust-clippy#2604
and rust-lang/cargo#6664

@christophermaier
Copy link
Contributor

@raskchanky @baumanj Cool, thanks for all that. 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants