refactor(swarm)!: don't be generic over Transport
#3272
Conversation
| @@ -79,9 +79,8 @@ impl ExecSwitch { | |||
| } | |||
|
|
|||
| /// A connection `Pool` manages a set of connections for each peer. | |||
| pub struct Pool<THandler, TTrans> | |||
There was a problem hiding this comment.
what do you think of making this pub (crate) instead? I know that it doesn't change anything as pool is not public but it helps visually understand that the type is shared across the crate (as opposed to just being private, which it can't be). pool is also pub (crate).
(this suggestion also applies to PoolEvent)
There was a problem hiding this comment.
In general I think I'd prefer to not use pub(crate) but I could be convinced otherwise.
It would be nice if we can assume that:
- All modules are private
- pub types therefore don't leak into the public API
- pub(crate) is therefore a workaround because rhe module isn't private for some reason
I am not fully convinced it is a good idea though, might cause a lot of complexity.
There was a problem hiding this comment.
I wish Rust had a way to whitelist what is in the public API of a crate and fail to compile if something is being added.
There was a problem hiding this comment.
if we decide that every pub type follows the convention used in #3238 we could use pub (crate) only inside the the crate itself to distinguish from the actually pub types on the main lib.rs for example, one issue with the assumptions above for example is that we have pub mod but in the end no strong opinion, I can understand your POV as well and share the same frustrations.
Description
Ever since we moved
Poolintolibp2p-swarm, we always use it with the sameTransport:Boxed. It is thus unnecessary for us to be overly generic over what kind ofTransportwe are using. This allows us to remove a few type parameters from the implementation which overall simplifies things.This is technically a breaking change because I am removing a type parameter from two exported type aliases:
PendingInboundConnectionErrorPendingOutboundConnectionErrorThose have always only be used with
std::io::Errorin our API but it is still a breaking change.Notes
Discovered as part of working on #2824.
Links to any relevant issues
Open Questions
Change checklist