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

dcutr: fix clippy lints #2772

Merged
merged 3 commits into from
Jul 28, 2022
Merged

dcutr: fix clippy lints #2772

merged 3 commits into from
Jul 28, 2022

Conversation

elenaf9
Copy link
Contributor

@elenaf9 elenaf9 commented Jul 24, 2022

Description

Fix clippy warnings in libp2p-dcutr: new-without-default, single-match, large-enum-variant, redundant-pattern-matching and redundant-closure.

Copy link
Contributor

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

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

ACK modulo one thing.

@@ -324,7 +320,6 @@ impl NetworkBehaviour for Behaviour {

/// A [`NetworkBehaviourAction`], either complete, or still requiring data from [`PollParameters`]
/// before being returned in [`Behaviour::poll`].
#[allow(clippy::large_enum_variant)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally, I always allow this one because the Box is quite the ergonomic hit when pattern matching and the lint desc. also says that one would need to measure the performance benefit gained from boxing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that in some cases it makes more sense to allow this lint rather than using a Box.
That being said, in this case here we pass around the inbound_connect quite a bit without using it, and for all enums it appeared on this error was thrown. We can wrap it once in a Box and pass that around instead, without any ergonomic hit. The only place where inbound_connect is used is in

if let Some(Poll::Ready(result)) = self.inbound_connect.as_mut().map(|f| f.poll_unpin(cx)) {
self.inbound_connect = None;

where we have to use AsMut anyway to access the inner future.

@elenaf9 elenaf9 requested a review from mxinden July 26, 2022 16:58
Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Thanks Elena!

@mxinden mxinden merged commit 09c6908 into libp2p:master Jul 28, 2022
@elenaf9 elenaf9 deleted the dcutr/fix-clippy branch July 28, 2022 07:35
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.

3 participants