refactor(swarm)!: deprecate PollParameters where possible
#3153
Conversation
|
|
||
| /// Utility struct for tracking the external addresses of a [`Swarm`](crate::Swarm). | ||
| #[derive(Debug, Default, Clone)] | ||
| pub struct ExternalAddresses { |
There was a problem hiding this comment.
| pub struct ExternalAddresses { | |
| pub struct ExternalAddressTracker { |
Not a strong opinion, though this would imply that one has to actively interact / pass-events with/to it.
There was a problem hiding this comment.
I am not convinced that this is a better name. Can you elaborate what you don't like about the current one? :)
There was a problem hiding this comment.
The name ExternalAddresses does not imply to me as a user, that I need to pass the FromSwarm events into it. That said, I think ExternalAddresses is just fine.
There was a problem hiding this comment.
I would argue that this is only something a user needs to learn once and quite obvious from the implementation.
Most of the time, this name should convey that it holds the external addresses and only every so often, you need to understand how it works so I optimised for the common bit :)
|
This pull request has merge conflicts. Could you please resolve them @thomaseizinger? 🙏 |
…st-libp2p into 3124-deprecate-poll-parameters
|
This pull request has merge conflicts. Could you please resolve them @thomaseizinger? 🙏 |
thomaseizinger
changed the title
PollParameters where possiblePollParameters where possible
thomaseizinger
changed the title
PollParameters where possiblePollParameters where possible
|
This pull request has merge conflicts. Could you please resolve them @thomaseizinger? 🙏 |
|
This pull request has merge conflicts. Could you please resolve them @thomaseizinger? 🙏 |
|
|
||
| /// Utility struct for tracking the external addresses of a [`Swarm`](crate::Swarm). | ||
| #[derive(Debug, Default, Clone)] | ||
| pub struct ExternalAddresses { |
There was a problem hiding this comment.
The name ExternalAddresses does not imply to me as a user, that I need to pass the FromSwarm events into it. That said, I think ExternalAddresses is just fine.
) This patch deprecates 3 out of 4 functions on `PollParameters`: - `local_peer_id` - `listened_addresses` - `external_addresses` The addresses can be obtained by inspecting the `FromSwarm` event. To make this easier, we introduce two utility structs in `libp2p-swarm`: - `ExternalAddresses` - `ListenAddresses` A node's `PeerId` is always known to the caller, thus we can require them to pass it in. Related: libp2p#3124.
) This patch deprecates 3 out of 4 functions on `PollParameters`: - `local_peer_id` - `listened_addresses` - `external_addresses` The addresses can be obtained by inspecting the `FromSwarm` event. To make this easier, we introduce two utility structs in `libp2p-swarm`: - `ExternalAddresses` - `ListenAddresses` A node's `PeerId` is always known to the caller, thus we can require them to pass it in. Related: libp2p#3124.
Description
This patch deprecates 3 out of 4 functions on
PollParameters:local_peer_idlistened_addressesexternal_addressesThe addresses can be obtained by inspecting the
FromSwarmevent. To make this easier, we introduce two utility structs inlibp2p-swarm:ExternalAddressesListenAddressesA node's
PeerIdis always known to the caller, thus we can require them to pass it in.Related: #3124.
Notes
I am opening this PR to get an initial concept ACK.
It is still missing changelog entries and we might want to consider waiting for the introduction of
FromSwarmto be merged before merging this. That would allow us to then extendFromSwarm::NewExternalAddresswith theAddressScorethat is tracked in theSwarmwhich makesExternaAddressesactually feature-equal with the currentPollParameters::external_addresses.Links to any relevant issues
Open Questions
Change checklist
I have added tests that prove my fix is effective or that my feature works