Conversation
robshakir
commented
May 28, 2021
robshakir
commented
* (M) client/client.go
* (M) client/client_test.go
- Add support for a set of queues within the client that store
responses for modify streams.
- Add basic test coverage for the queueing logic - additional
test coverage to be added through integration with the
fake server.
sthesayi
left a comment
sthesayi
left a comment
There was a problem hiding this comment.
Just some minor comments.
| return fmt.Errorf("cannot open Modify RPC, %v", err) | ||
| } | ||
|
|
||
| go func() { |
There was a problem hiding this comment.
nit: would be better if this was a separate function but inline is fine too.
There was a problem hiding this comment.
I err towards keeping these in-line because of the context that they're writing to. Essentially, I don't think they're ever testable, or callable (safely) outside of the Connect function, since they should never be called twice on the same connection. This way we can add (and probably should add) some state in the client that says "are we connected" and if so set up this stream. I didn't see a use-case for a client that doesn't have a Modify stream.
|
Thanks for the review @sthesayi -- I'll merge when you've had a chance to look at the other PRs that this depends on. |
2 participants
