transport: make I/O mode features additive #2236
Conversation
djc
force-pushed
the
additive-transport
branch
2 times, most recently
from
291d5fd to
0e377f7
Compare
Byron
left a comment
Byron
left a comment
There was a problem hiding this comment.
Thanks so much for contributing this!
I am pleasantly surprised how well this works, and it clearly was a mistake to not start additively in the first place. After all, on the consuming side, one can just import the same type name (either async or sync), and existing code will work as before.
So even if this for some reason stops translating up the dependency tree, there is a net-win.
This was just a cursory review, but I think once I can have my rustdoc links back, I can finish it and get this merged.
| #[cfg(feature = "async-network-client")] | ||
| use gix_transport::client::async_io::Transport; | ||
| #[cfg(feature = "blocking-network-client")] | ||
| use gix_transport::client::blocking_io::Transport; |
There was a problem hiding this comment.
I am particularly interested to see how this is ever (of course not in this PR) becoming additive without code duplication. But maybe it doesn't have to be, as making the plumbing crates additive is already a win.
In any case, I am staying tuned 🤩.
There was a problem hiding this comment.
Yes, I think there will be a bit more duplication at the higher levels, hopefully that will still be acceptable.
There was a problem hiding this comment.
I also realise now that as long as callers can use traits, these will work perfectly due to maybe_async. So maybe, just maybe, this will work just fine.
djc
force-pushed
the
additive-transport
branch
from
0e377f7 to
c2050d0
Compare
Byron
left a comment
Byron
left a comment
There was a problem hiding this comment.
Fantastic work! Even after looking carefully at everything, I wasn't able to make any of my usual refactoring 😁, so merging as is.
2 participants
No description provided.