Skip to content

Commit 044ecba

Browse files
committed
code health: fix all places highlighted by linter
Changed error's suppression in check.yaml. The suppression of rule `errcheck` may be removed after fixing errors check in all methods with calling encodeXxx inside. See details below. The rules `structcheck` and `unused` highlight the 'hack' with using _msgpack struct{} `msgpack:",asArray"`. The workaround with `//nolint` is not the best way to suppress it, cause this comment gets rendered by godoc and there is no way to make it invisible. For details see golang/go#20925. Suppressed the highlighting of error check in all methods, having encodeXxx inside. For now these methods are not able to return any error cause of internal implementation of writer interface (see smallbuf.go). For future, if the implementation of writer will be changed, and there will be a need to check errors, we must think about how to say to compiler that the error check is 'unlikely'. Fixed the use of time package API, all places with calls of time.Now().Sub(). Now time package propose the explicit time.Until(). Replaced all calls of Errorf() with Fatalf() in tests. That change prevents nil dereferences below in the code and stops test function execution, where it is expected in tests. Suppressed the highlighting of all unused constants and functions (Rules structcheck,unused in golangci-lint). Fixed calling of Fatalf from non-testing goroutine in queue tests. It is not a valid way to stop test from another goroutine. Fixed gofmt-highlighted places in test_helpers/pool_helper.go and connection_pool/const.go. Added instructions to CONTRIBUTING.md how to run CI-linter locally. Closes #142 Closes #150
1 parent e9b9ba1 commit 044ecba

18 files changed

+201
-164
lines changed

.github/workflows/check.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,4 @@ jobs:
3939
- name: golangci-lint
4040
uses: golangci/golangci-lint-action@v2
4141
with:
42-
args: --issues-exit-code=0 -E gofmt
42+
args: -E gofmt -D errcheck,unused,structcheck

CONTRIBUTING.md

+7
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,13 @@ For example, for running tests in `multi`, `uuid` and `main` packages, call
3838
make test-multi test-uuid test-main
3939
```
4040

41+
To check if the current changes will pass the linter in CI, install
42+
golnagci-lint from [sources](https://golangci-lint.run/usage/install/)
43+
and run it with next flags:
44+
```bash
45+
golangci-lint run -E gofmt -D errcheck,structcheck,unused test_helpers
46+
```
47+
4148
## Code review checklist
4249

4350
- Public API contains functions, variables, constants that are needed from

connection.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ func (conn *Connection) createConnection(reconnect bool) (err error) {
495495
conn.notify(ReconnectFailed)
496496
reconnects++
497497
conn.mutex.Unlock()
498-
time.Sleep(now.Add(conn.opts.Reconnect).Sub(time.Now()))
498+
time.Sleep(time.Until(now.Add(conn.opts.Reconnect)))
499499
conn.mutex.Lock()
500500
}
501501
if conn.state == connClosed {
@@ -688,7 +688,7 @@ func (conn *Connection) newFuture(requestCode int32) (fut *Future) {
688688
*pair.last = fut
689689
pair.last = &fut.next
690690
if conn.opts.Timeout > 0 {
691-
fut.timeout = time.Now().Sub(epoch) + conn.opts.Timeout
691+
fut.timeout = time.Until(epoch) + conn.opts.Timeout
692692
}
693693
shard.rmut.Unlock()
694694
if conn.rlimit != nil && conn.opts.RLimitAction == RLimitWait {
@@ -796,9 +796,9 @@ func (conn *Connection) timeouts() {
796796
return
797797
case <-t.C:
798798
}
799-
minNext := time.Now().Sub(epoch) + timeout
799+
minNext := time.Until(epoch) + timeout
800800
for i := range conn.shard {
801-
nowepoch = time.Now().Sub(epoch)
801+
nowepoch = time.Until(epoch)
802802
shard := &conn.shard[i]
803803
for pos := range shard.requests {
804804
shard.rmut.Lock()
@@ -825,7 +825,7 @@ func (conn *Connection) timeouts() {
825825
shard.rmut.Unlock()
826826
}
827827
}
828-
nowepoch = time.Now().Sub(epoch)
828+
nowepoch = time.Until(epoch)
829829
if nowepoch+time.Microsecond < minNext {
830830
t.Reset(minNext - nowepoch)
831831
} else {

0 commit comments

Comments
 (0)