Quit event #1523
Quit event #1523
Conversation
| s.done <- true | ||
| close(s.done) |
There was a problem hiding this comment.
This doesn't look quite right. As the done channel is non-blocking, it wouldn't wait for the reader to consume the true event, and just immediately close the channel. That of course, indicates the quite event, by waking up the channel, but most likely the client will not retrieve the true.
I think that is okay though. And we should just remove line 69 (and the buffer size of 1 on the channel).
|
|
||
| func (r *Resolver) Quit(ctx context.Context) (<-chan bool, error) { | ||
| logging.Debug("Quit resolver") | ||
| return r.done, nil |
There was a problem hiding this comment.
As you are returning the r.done channel which is the same for every subscriber to this quit event, at most one of the subscribers would receive the true value. But probably none of them (see other comment). All of them would wake up, which we could argue is enough. In that case I would change the return channel to <-chan struct{} though.
If you actually want to return a true event on the channel, I think something like this would work here:
| return r.done, nil | |
| res := make (chan bool) | |
| go func() { | |
| <-r.done | |
| res <- true | |
| }() | |
| return res, nil |
https://www.pivotaltracker.com/story/show/179151639