Channel Detach #298
Channel Detach #298
Conversation
sacOO7
commented
Mar 16, 2021
sacOO7
commented
- Fixes RTL5: Channel.detach #212
|
Most of the spec items are covered as a part of tests, till pending/skipped spec test item test is written, you can take a look @tcard |
removed error channel
|
@sacOO7 Unfortunately this PR now includes multiple commits that seem to have been rebased from |
sacOO7
force-pushed
the
fix/212-channel-detach
branch
from
ddcccfb to
e1dbf75
Compare
…dated tests for connection closing and failed
| }, nil | ||
| } | ||
| } | ||
|
|
||
| func (c ConnTransitioner) failOnIntercept(msg <-chan *proto.ProtocolMessage, cancelIntercept func()) connTransitionFunc { |
There was a problem hiding this comment.
We have a similar finishConnecting function that takes an error; if nil, finishConnecting transitions to CONNECTED; if non-nil, it transitions to FAILED.
I suggest doing the same with finishClosing. It took me a while to understand what failOnIntercept does. I believe it would be easier to understand if it was made clear it's supposed to be called in the context of having sent a CLOSE and being waiting a response.
There was a problem hiding this comment.
I believe, finishConnecting transitions to disconnected if error is passed as param
There was a problem hiding this comment.
True. Anyway, that's besides the point of the suggestion, really.
|
Besides a few suggestions, this is the only feedback that needs addressing. |
|
Merging channel detach PR, since approved by both parties : ) |
