-
Notifications
You must be signed in to change notification settings - Fork 55
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
fix(yamux): doesn't work in a Relayv2 connection #979
Conversation
4f6ed3b
to
8a58b09
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## unstable #979 +/- ##
=========================================
Coverage 83.19% 83.20%
=========================================
Files 91 91
Lines 15317 15328 +11
=========================================
+ Hits 12743 12753 +10
- Misses 2574 2575 +1
|
324d044
to
d282f52
Compare
Looks great, thanks a lot @lchenut. |
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.
LGTM ❇️
@@ -19,14 +19,22 @@ import ./helpers | |||
import std/times | |||
import stew/byteutils | |||
|
|||
proc createSwitch(r: Relay): Switch = | |||
result = SwitchBuilder.new() | |||
proc createSwitch(r: Relay = nil, useYamux: bool = false): Switch = |
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.
r: Relay = nil
Should we not generally use an Option in such cases?
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.
For a test, I believe it's not critical. What are your thoughts?
Co-authored-by: Ludovic Chenut <[email protected]>
Co-authored-by: Ludovic Chenut <[email protected]>
Co-authored-by: Ludovic Chenut <[email protected]>
Trying to use the circuit relay v2 with Yamux raises some issues: