-
Notifications
You must be signed in to change notification settings - Fork 1
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
Reorganize #1
Reorganize #1
Conversation
// Developer Guide: Logic for packaging | ||
|
||
// A trait for a encoder, developers should implement this trait and pass it to _write_to_outbound | ||
pub trait Encoder { |
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.
Are we planning on leaving encoder a trait
(Go: interface
) for the developers to implement? Should this be a V0 specific thing or is it good for future versions as well?
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.
Also encoder sounds a little bit too primitive (For me this name implies a stateless algorithm implementation). TLS, Shadowsocks (as transport), SSH and more, these all could be implemented with this model, so I would suggest a name change.
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.
Yeah I was having a hard time coming up with a good descriptor for this as well. My issue is similar in that things like encode/decode
or obscure/reveal
do not capture the ability to handshake and maintain a stream. We could do something like StreamEncoder
or StatefulEncoder
but those sound like the require more.
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.
Naming is so hard 😄
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.
Overall looks good to me.
Btw here's the proposed API for v0plus: https://github.com/gaukas/water_wasm-rs/tree/v0plus
I think it would be meaningful for us to also include v0plus, which is more transport-ful (instead of v0) in addition to v1, which is very much automated.
I can start a branch for |
Thank you so much for doing this! It looks great! If we all like this I guess we can merge it and I'll add some local changes I made previously to the new structure. |
This PR is an attempt to centralize our efforts to start reducing the amount of code that we re-write wrt. the rust runner (and example client applications), the water bin library, and the water binaries themselves.
This will help as we start settling in on the API so we can reduce the code required to implement each part.
I am making this PR first in preparation for future PRs like adding the hexencoder water bin example, a macro library to simplify writing water bins, and other example clients in rust.
One piece that I am not entirely clear on - several of the packages import an unknown trait
std::os::wasi::Prelude::FromRawFd
which my compiler did not recognize and which worked fine when replaced with an import for thestd::os::fd::FromRawFd
trait.