add new 'WithErrors' constructors #28
Conversation
| func NewSenderFromDevice(d *api.Device, errors chan<- error, cfg *Config) (*Sender, error) { | ||
| client := cfg.client() | ||
| return cfg.start(client, d, errors) | ||
| } | ||
|
|
||
| // NewSenderFromDeviceWithErrors returns a Sender and an error channel for an existing Device |
There was a problem hiding this comment.
Would be nice if the other public constructors also got this documentation
lib_test.go
Outdated
| }, | ||
| }, config) | ||
|
|
||
| assert.NotNil(t, s) |
There was a problem hiding this comment.
One of my pet peeves is that assert does not stop the test. So if s is nil here the test will fail but will continue to run. And then we'll see a nil pointer access down below when we call s.Send() and that just makes it harder to figure out what's actually broken from the noisier test output.
It's better to use require for assertions where you would want to fail the test and stop immediately.
Eg.: require.NotNil(t, s)
You don't have to change it if you don't want to. Just trying to get the word out.
There was a problem hiding this comment.
Funny you mention this. I first learned about require while reading a PR comment of yours from way back and started using it then. I like it a lot. Think I just got a bit lazy here.
I'll sharpen these tests up a bit.
This PR adds two new constructors which are variations of existing ones. These new functions return an error channel rather than accepting one which allows callers to not need to worry about closing the returned channel themselves.