-
Notifications
You must be signed in to change notification settings - Fork 60
Description
Describe the feature request
I would like to make WithListener
more strict about its parameter, requiring the consumer to implement some of the listener interfaces.
The rationale being: it's less work to implement a NOOP interface than it is to debug code that silently doesn't do what it's told.
Background
Currently, when using WithListener()
, it is possible to introduce silent errors by not implementing any of the listener interfaces. The sync()
goroutine will be started, but none of the listeners will be set.
This is presumably intentional, so that one can turn the sync on without having to actually implement anything (WithListener(struct{}{})
), but it is also extremely error-prone.
The following snippet showcases how easy it is to miss this:
func init() {
unleash.Initialize(
unleash.WithListener(unleashListener{}),
)
}
var _ unleash.RepositoryListener = (*unleashListener)(nil)
type unleashListener struct {}
func (*unleashListener) OnReady() {
log.Print("ready")
}
Note the interface-mismatch: we test against a pointer (*unleashListener
) but pass a copy (unleashListener{}
).
Solution suggestions
We could trivially check if any listener interface is implemented in NewClient
, but that would be a tricky compatibility breakage: old code would compile, but not run.
Maybe it would be better to break the API completely and change ConfigOption
into the following?
type ConfigOption func(*configOption) error
With this, we could move a lot of the validation code currently living in NewClient
to its respective option.
WDYT?
Metadata
Metadata
Assignees
Labels
Type
Projects
Status