Skip to content

Commit

Permalink
code health: fix all places highlighted by linter
Browse files Browse the repository at this point in the history
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
  • Loading branch information
vr009 committed Apr 28, 2022
1 parent e9b9ba1 commit 633e0cc
Show file tree
Hide file tree
Showing 18 changed files with 213 additions and 185 deletions.
10 changes: 9 additions & 1 deletion .github/workflows/check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,12 @@ jobs:
- name: golangci-lint
uses: golangci/golangci-lint-action@v2
with:
args: --issues-exit-code=0 -E gofmt
# The suppression of the rule `errcheck` may be removed after adding
# errors check in all methods calling EncodeXxx inside.
# For now those methods are not even able to return any error
# cause of internal implementation of writer interface (see smallbuf.go).
#
# The `//nolint` workaround was not the acceptable way of warnings suppression,
# cause those comments get rendered in documentation by godoc.
# See https://github.com/tarantool/go-tarantool/pull/160#discussion_r858608221
args: -E gofmt -D errcheck
7 changes: 7 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ For example, for running tests in `multi`, `uuid` and `main` packages, call
make test-multi test-uuid test-main
```

To check if the current changes will pass the linter in CI, install
golnagci-lint from [sources](https://golangci-lint.run/usage/install/)
and run it with next flags:
```bash
golangci-lint run -E gofmt -D errcheck
```

## Code review checklist

- Public API contains functions, variables, constants that are needed from
Expand Down
12 changes: 6 additions & 6 deletions connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ type connShard struct {
bufmut sync.Mutex
buf smallWBuf
enc *msgpack.Encoder
_pad [16]uint64
_pad [16]uint64 //nolint: unused,structcheck
}

// Greeting is a message sent by Tarantool on connect.
Expand Down Expand Up @@ -495,7 +495,7 @@ func (conn *Connection) createConnection(reconnect bool) (err error) {
conn.notify(ReconnectFailed)
reconnects++
conn.mutex.Unlock()
time.Sleep(now.Add(conn.opts.Reconnect).Sub(time.Now()))
time.Sleep(time.Until(now.Add(conn.opts.Reconnect)))
conn.mutex.Lock()
}
if conn.state == connClosed {
Expand Down Expand Up @@ -688,7 +688,7 @@ func (conn *Connection) newFuture(requestCode int32) (fut *Future) {
*pair.last = fut
pair.last = &fut.next
if conn.opts.Timeout > 0 {
fut.timeout = time.Now().Sub(epoch) + conn.opts.Timeout
fut.timeout = time.Until(epoch) + conn.opts.Timeout
}
shard.rmut.Unlock()
if conn.rlimit != nil && conn.opts.RLimitAction == RLimitWait {
Expand Down Expand Up @@ -796,9 +796,9 @@ func (conn *Connection) timeouts() {
return
case <-t.C:
}
minNext := time.Now().Sub(epoch) + timeout
minNext := time.Until(epoch) + timeout
for i := range conn.shard {
nowepoch = time.Now().Sub(epoch)
nowepoch = time.Until(epoch)
shard := &conn.shard[i]
for pos := range shard.requests {
shard.rmut.Lock()
Expand All @@ -825,7 +825,7 @@ func (conn *Connection) timeouts() {
shard.rmut.Unlock()
}
}
nowepoch = time.Now().Sub(epoch)
nowepoch = time.Until(epoch)
if nowepoch+time.Microsecond < minNext {
t.Reset(minNext - nowepoch)
} else {
Expand Down
Loading

0 comments on commit 633e0cc

Please sign in to comment.