Remove clippy allow directives/or provide reasons #1388
Labels
Comments
bors bot
pushed a commit
that referenced
this issue
Jul 23, 2020
## Issue Addressed NA ## Proposed Changes Fixes most clippy warnings and ignores the rest of them, see issue #1388.
bors bot
pushed a commit
that referenced
this issue
Jul 23, 2020
## Issue Addressed NA ## Proposed Changes Fixes most clippy warnings and ignores the rest of them, see issue #1388.
bors bot
pushed a commit
that referenced
this issue
Jul 28, 2020
## Issue Addressed #1388 partially (eth2_libp2p & network) ## Proposed Changes TLDR at the end - *Complex types* are 3 on the handlers/Behaviours but the types are `Poll<ComplexType>` where `ComplexType` comes from the traits of libp2p. Those, I don't thing are worth an alias. A couple more were from using tokio combinators and were removed writing things the async way and using [`BoxFuture`](https://docs.rs/futures/0.3.5/futures/future/type.BoxFuture.html) - The *cognitive complexity*.. I tried to address those before (they come from the poll functions too) and tbh they are cognitively simpler to understand the way they are now. Moving separate parts to functions doesn't add much since that code is not repeated and they all do early returns. If moved those returns would now need to be wrapped in an Option, probably, and checked to be returned again. I would leave them like that but that's just preference. - *Too many arguments*: They are not easily put together in a wrapping struct since the parameters don't relate semantically (Ex: fn new with a log, a reference to the chain, a peer, etc) but some may differ. - *Needless returns* were indeed needless ## Additional Info TLDR: removed needless return, used BoxFuture and async, left the rest untouched since those lgtm
ghost
added
paulhauner
changed the title
|
Closing due to inactivity and lots of changes to the code since it was created. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
In the last clippy cleanup (pull request #1385) some unresolved warnings got ignored via
#[allow...]directives. For each of them and all the other allow directives currently in the code base we should either remove them or comment on why the warnings should get ignored.A list of ignored warnings grouped by modules from pull request #1385 is (this list does not consider allow directives added before #1385):
beacon_node/storebeacon_node/networkbeacon_node/eth2_libp2pbeacon_node/beacon_chainbeacon_node/networkbeacon_node/clientreturnstatementbeacon_node/network: I could simply remove those returns, but I don't like it since if this function gets extended at some point the returns would be important. So I am not sure how to handle this (ignore the warning for the whole function?)Steps to resolve
Either remove or comment on all uncommented
#[allow...]directives in the code base.The text was updated successfully, but these errors were encountered: