-
Notifications
You must be signed in to change notification settings - Fork 163
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
refactor(iroh-net)!: remove the magicsock module from the public api #2247
refactor(iroh-net)!: remove the magicsock module from the public api #2247
Conversation
I don't have time for a thorough review, but I very much agree with this, if only to reduce the confusion when talking about MagicEndpoint and MagicSocket.
Didn't know this. Nice! |
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.
one change please, other than that LGTM
Description
magicsock
main's component, theMagicSock
is not useful on it's own. Having it on our public API forces us to treat as breaking changes modifications that for aMagicEndpoint
user are implementation details. To avoid this, this PR makes the following changes:MagicSock
aspub use
magicsock
frommagic_endpoint
(so that the types involved in the prev change can be used frommagic_endpoint
directly)magic_endpoint
module to not include theMagicSock
and make it more expressive at the same time.MagicSock
had a bunch of methods that were simply methods ofInner
re-exposed. Now thatMagicSock
is private, we can get rid of those all and simply useDeref
toInner
without exposingInner
via trait associated types.MagicSock
tomagicsock::Handle
which is what it really is, leaving only the new and close methods, which are the ones that handle creation and destruction of the associated tasks.MagicSock
tomsock
, which I think aligns better to new naming as is more readable/clear.Timer
code. The removed parts were considered used because they were publicly accessible. Becoming private showed parts that were dead code.local_addr
no longer returns an error (it seems to be it never did), but it became more clear in the process of accessing theMagicSock
(Inner
) methods viaDeref
.MagicEndpoint::local_addr
no longer returns an errorBreaking Changes
magicsock
is no longer publiciroh_net::magicsock::ConnectionType
->iroh_net::magic_endpoint::ConnectionType
iroh_net::magicsock::ControlMsg
->iroh_net::magic_endpoint::ControlMsg
iroh_net::magicsock::ConnectionInfo
->iroh_net::magic_endpoint::ConnectionInfo
iroh_net::magicsock::ConnectionTypeStream
->iroh_net::magic_endpoint::ConnectionTypeStream
iroh_net::magicsock::DirectAddrInfo
->iroh_net::magic_endpoint::DirectAddrInfo
iroh_net::magicsock::LocalEndpointsStream
->iroh_net::magic_endpoint::LocalEndpointsStream
iroh_net::magicsock::MagicSock
no longer accessible. UseMagicEndpoint
instead.iroh_net::magicsock::Options
no longer public. Used internally only.iroh_net::magicsock::PacketSplitIter
no longer public. This is an implementation detail of internal use only.iroh_net::magicsock::Timer
no longer public. This is an implementation detail of internal use only.iroh_net::magicsock::UdpSocket
no longer public. This is an implementation detail of internal use only.Notes & open questions
ended up using
cargo public-api -p iroh-net diff
to get what was added and make some of those not-for-public enums and structs private again. While really useful, checking this is very annoying. Hopefully we don't have to do it often.Change checklist
Tests if relevant.