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

core/: Remove TInEvent and TOutEvent #2183

Merged
merged 14 commits into from
Aug 11, 2021
Merged

Conversation

mxinden
Copy link
Member

@mxinden mxinden commented Aug 6, 2021

TInEvent and TOutEvent are implied through THandler and thus
superflucious. Both are removed in favor of a derivation through
THandler.

- Removes the `Swarm` type alias, renaming `ExpandedSwarm` to `Swarm`.
- Remove `TInEvent`, `TOutEvent` and `THandler` trait parameters on
`Swarm`, instead deriving them through `TBehaviour`. Move derive logic
to separate type aliases.
- Simplify trait bounds on `Swarm` main `impl` and `Stream` `impl`.
TInEvent and TOutEvent are implied through THandler and thus
superflucious. Both are removed in favor of a derivation through
THandler.
@mxinden mxinden marked this pull request as ready for review August 9, 2021 16:04
@mxinden
Copy link
Member Author

mxinden commented Aug 10, 2021

@thomaseizinger given your review on #2182, could you take a look at this patch as well?

@mxinden mxinden mentioned this pull request Aug 10, 2021
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.

Concept ACK

I like the reduction in type parameters, should make it easier to iterate on libp2p-core. Nice work!

@@ -2,6 +2,10 @@

- Update dependencies.

- Implement `Debug` for `RelayHandlerEvent` and `RealyHandlerIn`. See [PR 2183].
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor typo RealyHandlerIn.

Suggested change
- Implement `Debug` for `RelayHandlerEvent` and `RealyHandlerIn`. See [PR 2183].
- Implement `Debug` for `RelayHandlerEvent` and `RelayHandlerIn`. See [PR 2183].

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. Fixed in 6435bd7.

@@ -2,6 +2,10 @@

- Update dependencies.

- Implement `Debug` for `RequestResponseCodec` and `RequestProtocol`. See [PR 2183].
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that accurate? The Debug impl of RequestProtocol already existed, just now it is hand-rolled.

And I can't see a Debug impl for RequestResponseCodec itself.

Is this a leftover from an earlier iteration?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. Addressed in d78576c.

@@ -101,11 +101,11 @@ pub use select::{IntoProtocolsHandlerSelect, ProtocolsHandlerSelect};
/// continue reading data until the remote closes its side of the connection.
pub trait ProtocolsHandler: Send + 'static {
/// Custom event that can be received from the outside.
type InEvent: Send + 'static;
type InEvent: Debug + Send + 'static;
Copy link
Contributor

Choose a reason for hiding this comment

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

If you would reference these as fmt::Debug, the imports would be slightly cleaner.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am fine either way. Applied suggestion in 4576356.

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.

2 participants