chore: Enable golangci-lint and fix errors #554
Conversation
stevenh
force-pushed
the
golangci-lint
branch
from
6f64305 to
459b808
Compare
stevenh
force-pushed
the
golangci-lint
branch
2 times, most recently
from
e7e85d4 to
41dd401
Compare
Enable golangci-lint checking in travis and fix errors identified by it.
stevenh
force-pushed
the
golangci-lint
branch
from
41dd401 to
96e5580
Compare
activeConn.Close now returns any error from operations it performs instead of always returning nil.
|
@darkliquid @danmrichards could I get a review on this please |
| if err := c.Conn.Send("SUBSCRIBE", channel...); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Changes like this seem quite dangerous. While this is the correct behaviour, I do worry about the effect on applications using this package depending on the current behaviour of flush the connection regardless of the outcome of the send. Potentially this warrants a minor or even major version bump depending on how impactful this could be.
There was a problem hiding this comment.
My take on this particular case is that the Flush should be returning an error if the Send failed i.e. if Send failed Flush should also. Thoughts?
There was a problem hiding this comment.
Yeah, digging into it a bit more, conn is using a bufio.Writer for writes, whose behaviour described here https://golang.org/pkg/bufio/#Writer indicates that should a Write call to it fail (which is what Send will ultimately end up doing) then subsequent calls to Flush should also fail, so yeah, I think it's fine as it shouldn't actually result in a change in behaviour anyway.
| c.Do("PUBLISH", "c2", "world") | ||
| c.Do("PUBLISH", "c1", "goodbye") | ||
| if _, err = c.Do("PUBLISH", "c1", "hello"); err != nil { | ||
| fmt.Println(err) |
There was a problem hiding this comment.
is there a reason for using fmt.Println here over say t.Log()?
There was a problem hiding this comment.
It's because this is an example not a test, so trying to be more real.
Enable golangci-lint checking in travis and fix errors identified by it.