magic-wormhole.rs
magic-wormhole.rs copied to clipboard
Send client version with bind message
This PR sets the client_version
field in the bind
message (see magic-wormhole/magic-wormhole/src/wormhole_rendezvous.py:182). So far this is only being used by the Python and Wormhole-William (Go) clients.
It will use the following mapping:
implementation | rust |
version | <package-version> |
Codecov Report
Patch coverage: 100.00
% and project coverage change: +0.01
:tada:
Comparison is base (
46eceb0
) 43.73% compared to head (00842fb
) 43.74%.
Additional details and impacted files
@@ Coverage Diff @@
## master #176 +/- ##
==========================================
+ Coverage 43.73% 43.74% +0.01%
==========================================
Files 18 18
Lines 2552 2558 +6
==========================================
+ Hits 1116 1119 +3
- Misses 1436 1439 +3
Impacted Files | Coverage Δ | |
---|---|---|
src/core.rs | 78.91% <100.00%> (+0.38%) |
:arrow_up: |
src/core/server_messages.rs | 68.57% <100.00%> (+0.92%) |
:arrow_up: |
... and 3 files with indirect coverage changes
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
This field does not seem to be specified in the protocols. We should fix that
Using lifetimes here has the effect of avoiding about two allocations in total and is total overkill. But if you really want to have a borrowed version of the struct, then use Cow
instead please.
I'm new to Rust and need to find out how to avoid lifetimes then. Should I prefer to use String
instead of &str
to achieve that?
Just clone liberally. And if you find yourself cloning too much, wrap in a reference counter to make it cheaper.
One of the key lessons most beginners eventually learn is that Rust gives you the possibility to be efficient, but it's not always necessary/productive to make use of that. (Incidentally, I think this is also where a lot of the "fighting the borrow checker" meme comes from)
The solution is now hopefully clean enough.
Yes, it is.
Unless this PR is urgent, I'd like to discuss the client-version
field in the protocol before merging this. Depending on what the values mean, I'd personally prefer CLIENT_NAME = "wormhole-rs";
over "rust"
. On the other hand, there may exist multiple clients using this library, should the value reflect that or not?
On the other hand, there may exist multiple clients using this library, should the value reflect that or not?
That would be a future update to enable overriding this information by the caller, like in https://github.com/psanford/wormhole-william/blob/master/rendezvous/options.go.
https://github.com/magic-wormhole/magic-wormhole-protocols/issues/35