Fix lint errors #78
Fix lint errors #78
Conversation
|
@fho I left some TODO statements to figure out what to do in some error situations. |
|
@lukebakken I'm looking into handling the error returned by |
|
@lukebakken that worked, here is the PR: #80 |
|
Tests pass but there are still quite a few TODOs. |
|
For now I think we can review this and address the I will squash before merging of course. |
There was a problem hiding this comment.
As you already wrote, except for the panic() which should be avoided in libraries, the remaining changes look good to me. Thanks @lukebakken !
There was a problem hiding this comment.
I think we should not add new panic calls to the master branch.
Applications would now crash instead of ignoring errors in places where it seems to not have caused issues so far.
We could keep the linter errors on those lines (+ enable that golangci-lint only complains for new issues on PRs again) or add TODO + // nolint lines, merge this PR and do #81 later.
This would ensure that the version in master is as stable as before and a new release could be created anytime.
|
@fho we can do that. One of the main reasons for this project to be created was that versioning was not being used in https://github.com/streadway/amqp. If you're using |
Start by removing some dead code.
types: remove unused typeSet type
The type was not used at all, remove it.
This was found by the unused linter:
types.go:269:6: type `tagSet` is unused (unused)
tests: fix: declaration of unused value
This fixes:
integration_test.go:1941:2: SA4006: this value of `ch` is never used
(staticcheck)
tests: fix: check returned errors when getting a channel
This fixes:
integration_test.go:808:8: ineffectual assignment to err (ineffassign)
pub, err := c.Channel()
^
integration_test.go:915:8: ineffectual assignment to err (ineffassign)
pub, err := c1.Channel()
^
integration_test.go:916:8: ineffectual assignment to err (ineffassign)
sub, err := c2.Channel()
^
fix warnings from gosimple
This fixes:
client_test.go:75:5: S1004: should use !bytes.Equal(b, in) instead (gosimple)
if bytes.Compare(b, in) != 0 {
integration_test.go:1009:7: S1004: should use !bytes.Equal(msg.Body, fixture.Body) instead (gosimple)
if bytes.Compare(msg.Body, fixture.Body) != 0 {
integration_test.go:1928:6: S1004: should use !bytes.Equal(want, got.Body) instead (gosimple)
if bytes.Compare(want, got.Body) != 0 {
uri.go:58:5: S1002: should omit comparison to bool constant, can be simplified to `strings.Contains(uri, " ")` (gosimple)
if strings.Contains(uri, " ") == true {
^
write.go:421:25: S1030: should use buf.String() instead of string(buf.Bytes()) (gosimple)
return writeLongstr(w, string(buf.Bytes()))
tests/TestIntegrationChannelClosing: don't call func that does nothing
The TestIntegrationChannelClosing was defining a function that was doing
nothing and calling it 2x, remove this.
ci: report all existing golangci-lint issues
Instead of only reporting golangci-lint issues that are new in a
pull-request, report all existing ones.
All open issues are fixed, afterwards having 0 issues can be norm.
Add error check with TODO
golangci-lint fixes
client_test.go is now lint-free
examples_test.go is now lint-free
reconnect_test.go is now lint-free
connection_test.go is now lint-free
connection_test.go is now lint-free
Starting de-linting on integration_test.go
Fix TestRaceBetweenChannelShutdownAndSend
connection.go is lint-free
removing lint from integration_test.go. Ignore net.OpError in heartbeater
remove more lint from integration_test.go
remove more lint from integration_test.go
Fix deadlocked test
Fix test
Variable naming
Do not panic
Bump and simplify some CI settings
Start by removing some dead code.
@fho I have added you as a collaborator on my fork. Please feel free to push to this branch.