Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Make clippy _a little_ more annoying #10570

Merged
merged 15 commits into from
Jan 5, 2022
Merged

Make clippy _a little_ more annoying #10570

merged 15 commits into from
Jan 5, 2022

Conversation

gilescope
Copy link
Contributor

We've been running clippy via the CI for a few months and it all seems to be going smoothly, but Clippy is keeping a very low profile. We don't want it to be a thorn in our side but it's probably ok to dial clippy up a little.

This PR turns on the Complexity category lints ( https://rust-lang.github.io/rust-clippy/master/ ) except 15 which were too annoying.

If you are puzzled over a code change just ask and I'll let you know the rational behind it.

There's a couple of methods where unused lifetimes have been removed - please shout out if that's not desirable for backward compatability reasons.

Let the bikeshedding of clippy complexity lints begin!

@gilescope gilescope added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C3-medium PR touches the given topic and has a medium impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Dec 30, 2021
bkchr
bkchr suggested changes Dec 30, 2021
View reviewed changes
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

If you do this, I expect that you fix the code properly and not sprinkle around clippy annotations. For most of the stuff they are really not needed, because the code can just be changed easily.

If we do this, we either do it right or we just don't do it.

client/db/src/bench.rs Outdated Show resolved Hide resolved
client/cli/src/arg_enums.rs Outdated Show resolved Hide resolved
bin/node/inspect/src/lib.rs Outdated Show resolved Hide resolved
client/executor/src/wasm_runtime.rs Outdated Show resolved Hide resolved
client/executor/wasmtime/src/instance_wrapper.rs Outdated Show resolved Hide resolved
client/service/src/task_manager/tests.rs Outdated Show resolved Hide resolved
client/state-db/src/noncanonical.rs Outdated Show resolved Hide resolved
client/tracing/src/lib.rs Outdated Show resolved Hide resolved
primitives/keystore/src/testing.rs Outdated Show resolved Hide resolved
@bkchr bkchr removed the C3-medium PR touches the given topic and has a medium impact on builders. label Dec 30, 2021
gilescope and others added 7 commits December 31, 2021 09:46
@gilescope @bkchr
Update client/cli/src/arg_enums.rs
591ccc1 
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
@gilescope @bkchr
Update bin/node/inspect/src/lib.rs
f371bf5 
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
@gilescope @bkchr
Update primitives/keystore/src/testing.rs
c228e82 
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
@gilescope @KiChjang
Update frame/elections/src/lib.rs
d2bc4b0 
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
@gilescope @KiChjang
Update primitives/npos-elections/fuzzer/src/reduce.rs
d68365f 
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
@gilescope
Incorporating feedback
87548dd
@gilescope
Merge branch 'master' into giles-make-clippy-annoying-again
4597359
@gilescope gilescope added the C1-low PR touches the given topic and has a low impact on builders. label Dec 31, 2021
wigy-opensource-developer
wigy-opensource-developer reviewed Dec 31, 2021
View reviewed changes
Copy link
Contributor

@wigy-opensource-developer wigy-opensource-developer left a comment

Choose a reason for hiding this comment

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

Nice. My comments are just informational, do not worry too much about them.

client/executor/wasmtime/src/host.rs Outdated Show resolved Hide resolved
frame/authorship/src/lib.rs Outdated Show resolved Hide resolved
frame/authorship/src/lib.rs Outdated Show resolved Hide resolved
frame/support/src/storage/migration.rs Outdated Show resolved Hide resolved
frame/support/src/storage/migration.rs Outdated Show resolved Hide resolved
primitives/npos-elections/fuzzer/src/compact.rs Show resolved Hide resolved
primitives/runtime/src/generic/digest.rs Show resolved Hide resolved
primitives/timestamp/src/lib.rs Outdated Show resolved Hide resolved
primitives/transaction-storage-proof/src/lib.rs Outdated Show resolved Hide resolved
test-utils/runtime/src/lib.rs Outdated Show resolved Hide resolved
athei
athei reviewed Jan 3, 2022
View reviewed changes
frame/contracts/src/exec.rs Outdated Show resolved Hide resolved
@gilescope
Copy link
Contributor Author

The PR now includes no clippy annotations (and in fact removes one). @bkchr does the PR now address all your concerns?

@kianenigma kianenigma removed their request for review January 4, 2022 13:19
bkchr
bkchr approved these changes Jan 4, 2022
View reviewed changes
frame/support/src/traits/tokens/currency.rs Outdated Show resolved Hide resolved
frame/support/src/traits/tokens/currency.rs Outdated Show resolved Hide resolved
@gilescope gilescope requested a review from a team as a code owner January 5, 2022 13:46
@gilescope
Copy link
Contributor Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit a2ae8a5 into paritytech:master Jan 5, 2022
@shawntabrizi shawntabrizi mentioned this pull request Jan 5, 2022
Merged
@nazar-pc nazar-pc mentioned this pull request Jan 15, 2022
Merged
@DaviRain-Su DaviRain-Su mentioned this pull request Mar 10, 2022
Merged
@shawntabrizi shawntabrizi mentioned this pull request Mar 16, 2022
Closed
grishasobol pushed a commit to gear-tech/substrate that referenced this pull request Mar 28, 2022
@gilescope @bkchr
@KiChjang @grishasobol
Make clippy _a little_ more annoying (paritytech#10570)
15445d9 
* Clippy: +complexity

* Update client/cli/src/arg_enums.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update bin/node/inspect/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update primitives/keystore/src/testing.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update frame/elections/src/lib.rs

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

* Update primitives/npos-elections/fuzzer/src/reduce.rs

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

* Incorporating feedback

* No need for Ok

* Additional

* Needed slice

* Wigy's suggestions on less derefs

* fix count

* reverting changes brought in by option_map_unit_fn

* add --all-targets

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@gilescope @bkchr
@KiChjang @ark0f
Make clippy _a little_ more annoying (paritytech#10570)
8bbab7e 
* Clippy: +complexity

* Update client/cli/src/arg_enums.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update bin/node/inspect/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update primitives/keystore/src/testing.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update frame/elections/src/lib.rs

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

* Update primitives/npos-elections/fuzzer/src/reduce.rs

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

* Incorporating feedback

* No need for Ok

* Additional

* Needed slice

* Wigy's suggestions on less derefs

* fix count

* reverting changes brought in by option_map_unit_fn

* add --all-targets

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@KiChjang KiChjang KiChjang left review comments

@bkchr bkchr bkchr approved these changes

@athei athei athei approved these changes

@wigy-opensource-developer wigy-opensource-developer wigy-opensource-developer approved these changes

@andresilva andresilva Awaiting requested review from andresilva

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gilescope @wigy-opensource-developer @athei @KiChjang @bkchr