This repository has been archived by the owner on Jul 21, 2023. It is now read-only.
workflows: Run clippy during CI #40
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yuriks
force-pushed
the
yuriks/ci-clippy
branch
2 times, most recently
from
db12042
to
569633a
Compare
This is already done for some imports but not all. Run rustfmt with the `merge_imports`[1] option to uniformize them. Unfortunately this option is only available in nightly right now so it can't be added to `rustfmt.toml` yet. [1]: https://rust-lang.github.io/rustfmt/?version=master&search=nest#merge_imports
This is a grab-bag of minor fixes to address various different clippy lints.
This trait was only used to add helper methods to BatchReader and BatchWriter, however, it's clearer to just inline them into callers such that there is only one constructor for BatchReader and BatchWriter.
Maybe this struct should be restructured to not have the `H` and `P` parameters, but in the meantime this avoids restricting variance and tying them to the (unrelated) `'a` lifetime. `PhantomData<*const T>` is used which doesn't imply any ownership of data (unlike `T`) but also has no lifetimes associated with it (unlike `&'a T`).
These are warnings that are annoying to fix without re-structuring the existing code. Silence them so that we get a clean clippy output from now on.
yuriks
force-pushed
the
yuriks/ci-clippy
branch
from
569633a
to
17d95c4
Compare
tgeoghegan
approved these changes
Oct 13, 2020
|
Filed #48 to track future improvements to the GitHub Actions fail-fast behavior which I mentioned in the PR description. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I recommend reviewing commits individually.
This PR fixes or silences all the outstanding clippy lints in the default configuration, and enable clippy to run as a GitHub action. This also includes a few other unrelated cleanups I spotted along the way.
One trade-off here is that I configured clippy to run as part of our existing "build" job (just like the
cargo fmtcheck currently does). This is nice in that is re-uses the compilation cache that clippy generates (since it needs to actually do full type-checking with the compiler to work, it just doesn't do any codegen), but it also means that a clippy failure will abort the job before it attempts to do an actual compile. While clippy does report most error that the compiler itself does, it does miss a few things checked by the compiler in later stages. In this case the PR author would need to a edit-push-wait for CI cycle before the other errors will be surfaced by CI.To fix this we could either:
if:conditions on every single later build step.)If anyone thinks it would be worth it, I can implement either of these options and update the PR.