Skip to content
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: tacd: use only cargo clippy and not cargo check as well #41

Merged
merged 1 commit into from
Oct 6, 2023

Conversation

hnez
Copy link
Member

@hnez hnez commented Sep 27, 2023

The checks performed by running cargo check are a subset of what cargo clippy checks for (as indicated by cargo clippy [using cargo check in the background]):

https://github.com/rust-lang/rust-clippy/blob/c154754b74577906c5d55d57f7daeff02d6a33e7/src/main.rs#L59-L105

Running both is thus a waste of CPU cycles. Run cargo clippy only.

Copy link
Member

@KarlK90 KarlK90 left a comment

Choose a reason for hiding this comment

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

LGTM, we could probably run the tests after all lints have been satisfied to catch malformed code and save some more cycles.

The checks performed by running cargo check are a subset of what
cargo clippy checks for (as indicated by cargo clippy using cargo
check in the background [1]).

Running both is thus a waste of CPU cycles.

[1]: https://github.com/rust-lang/rust-clippy/blob/c154754b74577906c5d55d57f7daeff02d6a33e7/src/main.rs#L59

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
@hnez
Copy link
Member Author

hnez commented Oct 6, 2023

Running lints and tests in sequence would:

  • Increase our push-to-CI-done latency. Which is kind of annoying when e.g. updating a branch to the recent main.
  • Mean we would only see actual build problems after all lints are fixed.

I think these drawbacks make it not worth it.


I do however have another idea with regards to CI jobs:

We could also run the basic cargo clippy job with the current rust version we use in meta-lxatac to make sure we do not break compatibility with said version (which already happened and will happen again, for example when #42 is merged).

@hnez hnez merged commit 9d58ed2 into linux-automation:main Oct 6, 2023
6 of 7 checks passed
@hnez hnez deleted the no-cargo-check branch October 6, 2023 14:02
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