Skip to content

Commit de95e31

Browse files
committed
code health: fix all places highlighted by linter
Changed the warning's suppression in check.yaml. The suppression of the rule errcheck may be removed after adding errors check in all methods calling encodeXxx inside. See details below. Suppressed the highlighting lacks of error's check in all methods, having encodeXxx inside. For now those methods are not able to return any error due to internal implementation of writer interface (see smallbuf.go). For future, if the implementation of the 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' for keeping performance (If there will be any affect on it). Fixed the use of time package API in 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, where it is needed. That change prevents nil dereferences below in the code and stops the test execution, where it is expected in tests. Suppressed the highlighting of all unused constants and functions with //nolint comment. Fixed the calling of Fatalf() from non-testing goroutine in queue tests. It is not a valid way to stop test from another goroutine. Fixed the 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 de95e31

18 files changed

+213
-185
lines changed

.github/workflows/check.yaml

+9-1
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,12 @@ jobs:
3939
- name: golangci-lint
4040
uses: golangci/golangci-lint-action@v2
4141
with:
42-
args: --issues-exit-code=0 -E gofmt
42+
# The suppression of the rule `errcheck` may be removed after adding
43+
# errors check in all methods calling EncodeXxx inside.
44+
# For now those methods are not even able to return any error
45+
# cause of internal implementation of writer interface (see smallbuf.go).
46+
#
47+
# The `//nolint` workaround was not the acceptable way of warnings suppression,
48+
# cause those comments get rendered in documentation by godoc.
49+
# See https://github.com/tarantool/go-tarantool/pull/160#discussion_r858608221
50+
args: -E gofmt -D errcheck

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
46+
```
47+
4148
## Code review checklist
4249

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

connection.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ type connShard struct {
147147
bufmut sync.Mutex
148148
buf smallWBuf
149149
enc *msgpack.Encoder
150-
_pad [16]uint64
150+
_pad [16]uint64 //nolint: unused,structcheck
151151
}
152152

153153
// Greeting is a message sent by Tarantool on connect.
@@ -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.Since(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.Since(epoch) + timeout
800800
for i := range conn.shard {
801-
nowepoch = time.Now().Sub(epoch)
801+
nowepoch = time.Since(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.Since(epoch)
829829
if nowepoch+time.Microsecond < minNext {
830830
t.Reset(minNext - nowepoch)
831831
} else {

0 commit comments

Comments
 (0)