core/transport: Improve docs for Transport::address_translation
#2976
Conversation
elenaf9
force-pushed
the
transport-addr-translation
branch
from
3785dd8
to
de05313
Compare
|
Force-pushed to drop 3785dd8. I believe a changelog entry is not needed given that I just updated the docs? |
elenaf9
changed the title
Transport::address_translation, add default NoneTransport::address_translation
| /// so that e.g. the peer is observed at a different IP than the IP of the local | ||
| /// listening address. See also [`address_translation`][crate::address_translation]. | ||
| /// | ||
| /// Within [`libp2p::Swarm`](<https://docs.rs/libp2p/latest/libp2p/struct.Swarm.html>) this is |
There was a problem hiding this comment.
Is there a better way to link to items from crates that are not a dependency?
There was a problem hiding this comment.
I am not aware of a way. Maybe @thomaseizinger?
There was a problem hiding this comment.
Me neither but I also think it is kind of an odd thing to do. This crate shouldn't "know" about Swarm. Do we have to include this sentence here?
There was a problem hiding this comment.
Me neither but I also think it is kind of an odd thing to do. This crate shouldn't "know" about
Swarm. Do we have to include this sentence here?
Good point. But in practice transports are commonly used in a swarm, so I think its okay to mention this here. When I was looking at it for Quic it helped me to know how it is used in the swarm.
| /// so that e.g. the peer is observed at a different IP than the IP of the local | ||
| /// listening address. See also [`address_translation`][crate::address_translation]. | ||
| /// | ||
| /// Within [`libp2p::Swarm`](<https://docs.rs/libp2p/latest/libp2p/struct.Swarm.html>) this is |
There was a problem hiding this comment.
I am not aware of a way. Maybe @thomaseizinger?
thomaseizinger
changed the title
Transport::address_translationTransport::address_translation
Description
Transport::address_translation3785dd8: proposal: add default implementation forTransport::address_translationthat returnsNone.Links to any relevant issues
Motivated by a recent discussion in #2289 (comment) and a follow-up out of band discussion with @mxinden , that showed that the usage and relevance of this method is currently not well documented.
Open Questions
Does a defaultNoneimplementation forTransport::address_translationmake sense? It might help emphasizing that this is not relevant for all transports? Not sure about this myself, happy to revert that commit.Change checklist