Stateful eventbus #51
Conversation
autonat.go
Outdated
| emitUnknown, _ := h.EventBus().Emitter(new(event.EvtLocalRoutabilityUnknown), eventbus.Stateful) | ||
| emitPublic, _ := h.EventBus().Emitter(new(event.EvtLocalRoutabilityPublic), eventbus.Stateful) | ||
| emitPrivate, _ := h.EventBus().Emitter(new(event.EvtLocalRoutabilityPrivate), eventbus.Stateful) |
There was a problem hiding this comment.
How will this interact with a subscriber who subscribes to all three events? Won't they just be confused as to the current state?
Is there a reason we don't have a single emitter with multiple different state options, instead of three emitters?
There was a problem hiding this comment.
Depends on libp2p/go-libp2p-core#126
There was a problem hiding this comment.
We've agreed to merge all three types into a single type with an enum and replace all the emitters with a single emitter. @Stebalien is working on a PR for that and will fix this PR once this is done.
|
@willscott how stable is #41, and would you recommend me using it within the DHT testground plan? I'm planning on updating the test plan to use the latest proposed DHT changes, and they include a dependency on the latest version of go-libp2p-core (for the Reachability event) which is emitted in this PR. If you think I should rely on #41 then go for the merge and lmk so I can figure out which commit to depend on. Also, is there a reason why we shouldn't merge this and then rebase #41 on it? While it's a bit annoying that we're depending on a commit of go-libp2p-core instead of release it seems like we can just update the go.mod after we do a -core release. |
|
Closing as this has been absorbed in #41. |
@Stebalien
For libp2p/go-libp2p-kad-dht#469 (comment)