Rename Dial to DialStream and DialPacket #150
Replies: 5 comments
-
One potential downside to this is that it would prevent the use of Generics. Generics has the potential to reduce code duplication, and I played with it a bit in a fork. Generics in Go are pretty bad though, and I ran into a lot of issues with the type inference and dealing with it was even more of a pain. |
Beta Was this translation helpful? Give feedback.
-
I'm OK with the duplicated logic, but I would prefer to do it early than late because it is a big breaking change. Another option is to add new interfaces, and gradually move old implementation to the new one. |
Beta Was this translation helpful? Give feedback.
-
I agree with doing it sooner rather than later. Though there are big PRs still under way to merge first. I agree with the adding new interfaces, if we were promising a stable API. The point of keeping it in v0 for now is to enable us to make breaking changes before we commit to a stable API, as we learn from real world usage. Having two competing Dial methods will be a pain, like it is with the standard library Dial/DialContext. |
Beta Was this translation helpful? Give feedback.
-
Implemented and released: https://github.com/Jigsaw-Code/outline-sdk/releases/tag/v0.0.12 |
Beta Was this translation helpful? Give feedback.
-
Currently StreamDialers and PacketDialers both have a Dial method. That means an object can't be both, but there are many situations where we want to be able to dial either streams or packets (e.g. a DNS resolver, a SOCKS proxy,...).
I propose we rename the Dial method to DialStream and DialPacket, so that an object can implement both.
The same applies to Endpoints. We need ConnectStream and ConnectPacket.
This is obviously a breaking change, but we don't yet promise API stability (we are still v0).
/cc @jyyi1
Beta Was this translation helpful? Give feedback.
All reactions