-
Notifications
You must be signed in to change notification settings - Fork 998
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
swarm/src/either: implement NetworkBehaviour
on Either
#2370
Conversation
Implement `NetworkBehaviour` on `either::Either<L, R>` where both L and R both implement `NetworkBehaviour`. Add NetworkBehaviour derive tests for Either and Toggle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @mexskican for picking this up!
Looks good to me, just a couple of smaller comments. Great job getting familiar with these somewhat complicated internals!
While reviewing I had the idea described in #2374. I would be interested in your opinion @mexskican.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
swarm/src/either.rs
Outdated
match self { | ||
Either::Left(a) => IntoEitherHandler::Left(a.new_handler()), | ||
Either::Right(b) => IntoEitherHandler::Right(b.new_handler()), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a pity rayon-rs/either#58 is not merged / released yet ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. THanks @mexskican for the follow-ups!
Closes #2353
Implement
NetworkBehaviour
oneither::Either<L, R>
where both Land R implement
NetworkBehaviour
.Add
NetworkBehaviour
derive tests for Either and Toggle