Enabe clippy on CI #1061
Merged
Enabe clippy on CI #1061
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
clippy emits two warnings of form: warning: redundant field names in struct initialization Remove the redundant field names.
clippy emits:
warning: this `else { if .. }` block can be collapsed
In this instance the code is more readable how it is, we should ignore
clippy.
Add compiler directive to quieten warning.
clippy emits a bunch of: warning: statics have by default a `'static` lifetime Remove the unnecessary 'static lifetimes.
clippy emits a bunch of: warning: digits grouped inconsistently by underscores We have a custom grouping elsewhere in this file 10_000_000_00 sats == 10 BTC Fix up all instances of large sats amount to uniformly using this format and add compiler directives where needed to shoosh clippy.
clippy emits: warning: this expression creates a reference which is immediately dereferenced by the compiler As suggested, remove the explicit reference.
clippy emits: warning: using `clone` on type `blockdata::transaction::OutPoint` which implements the `Copy` trait Remove unneeded call to `clone`.
clippy emits: warning: question mark operator is useless here As suggested, remove the `?` operator.
Add a clippy configuration file configuring the MSRV. Add a github actions job to run clippy on CI. Please note the job does _not_ use `--all-targets` because doing so causes: ``` error[E0554]: `#![feature]` may not be used on the stable release channel --> src/lib.rs:46:54 | 46 | #![cfg_attr(all(test, feature = "unstable"), feature(test))] ```
Closed
Add a `githooks` directory. Copy the sample pre-commit hook into it. Add a section to the README explaining how to take advantage of the githooks. The sample pre-commit hook includes checks for trailing whitespace that we are seeing occasionally in PRs. Done in preparation for adding a clippy githook.
We have a `pre-commit` githook, add to it a call to `cargo clippy`.
tcharding
force-pushed
the
06-23-enable-clippy-ci
branch
from
702aea8
to
e2e4650
Compare
apoelstra
approved these changes
Jun 23, 2022
|
I think we should be pretty liberal. It is better than our previous policy of never using clippy. |
ChallengeDev210
pushed a commit
to ChallengeDev210/rust-bitcoin
that referenced
this pull request
Aug 1, 2022
e2e4650 Add clippy to pre-commit githook (Tobin C. Harding) 820adc0 Add githooks directory (Tobin C. Harding) 668b37a Enable clippy on CI (Tobin C. Harding) a2a54b3 Remove unnecessary ? operator (Tobin C. Harding) 67ed8f6 Remove unneeded clone (Tobin C. Harding) eccd401 Remove unneeded reference (Tobin C. Harding) fd4239f Use custom digit grouping (Tobin C. Harding) acd551e Remove unnecessary 'static lifetime (Tobin C. Harding) 3102a48 Allow clippy::collapsible_else_if (Tobin C. Harding) 63f4ff6 Remove redundant field names (Tobin C. Harding) Pull request description: We are almost ready to enable clippy on CI! First clear a few remaining warnings from feature gated code. Then add a CI job to run clippy using `--all-features` (note no `--all-targets`). Next add githooks directory (this is the patch from https://github.com/rust-bitcoin/rust-bitcoin/pull/1044/commits). Finally add `cargo clippy` to the pre-commit hook. EDIT: I just realized the running of clippy could have been done in `test.sh` instead of as a github action. Does that matter? ACKs for top commit: apoelstra: ACK e2e4650 Kixunil: ACK e2e4650 Tree-SHA512: c78cb78973d3935e5be7908eec7d6ffaa7989724b3e29551b9fa2cb35df1f39574e31e5cc93fdfb32b35039ac9dac5c080ae4287a1e6979dd076bab56e6eda1b
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
We are almost ready to enable clippy on CI!
First clear a few remaining warnings from feature gated code. Then add a CI job to run clippy using
--all-features(note no--all-targets). Next add githooks directory (this is the patch from https://github.com/rust-bitcoin/rust-bitcoin/pull/1044/commits). Finally addcargo clippyto the pre-commit hook.EDIT: I just realized the running of clippy could have been done in
test.shinstead of as a github action. Does that matter?