Skip to content

Commit

Permalink
ci: enhanced linting (#158)
Browse files Browse the repository at this point in the history
Add a configuration file which makes it much more strict.

Fix shadowed error variables flagged by linter.

Remove CI timeout as its now present in the config.
  • Loading branch information
stevenh authored Oct 7, 2024
1 parent 93b3db4 commit 5d57bf3
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 8 deletions.
1 change: 0 additions & 1 deletion .github/workflows/golangci-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,3 @@ jobs:
uses: golangci/golangci-lint-action@aaa42aa0628b4ae2578232a66b541047968fac86 # v6.1.0
with:
version: v1.60.3
args: --timeout=3m
86 changes: 86 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
run:
timeout: 2m

linters-settings:
gosec:
excludes:
- G601 ## Implicit memory aliasing of items from a range statement - not possible in go 1.22.
cyclop:
max-complexity: 15
nestif:
min-complexity: 10
govet:
settings:
shadow:
strict: true
enable-all: true
nolintlint:
require-explanation: true
godot:
scope: all
nakedret:
max-func-lines: 0

linters:
enable-all: true
disable:
# Spammy / low value
- nonamedreturns
- varnamelen
- exhaustruct
- nlreturn
- wsl
- lll
- paralleltest
# Duplicate functionality.
- funlen
- gocognit
# Deprecated.
- execinquery
- gomnd
# Good but gets in the way too often.
- testpackage
# Unknown details about how Artemis works are flagged with TODO's.
- godox
# Seems to be broken.
- depguard
# Makes it messy for multiple optional tags.
- tagalign
# Not needed for go 1.22+.
- exportloopref
- errchkjson # Duplicate functionality for errcheck.

issues:
include:
- EXC0012
- EXC0014
exclude-rules:
# Exclude linters which aren't an issue in tests.
- path: _test\.go
linters:
- gochecknoglobals
- wrapcheck

# File mode permissions are fine for constants.
- text: "Magic number: 0o\\d+"
linters:
- mnd

# Field alignment in tests isn't a performance issue.
- text: fieldalignment
path: _test\.go

# Dynamic errors can provide useful context.
- text: "do not define dynamic errors, use wrapped static errors instead:"
linters:
- err113

# We need to use the `err` named return for error handling.
- text: 'named return "err" with type "error" found'
linters:
- nonamedreturns

# Interface casting is fine in mock.
- path: mock_test\.go
linters:
- forcetypeassert
2 changes: 1 addition & 1 deletion reaper.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func (r *reaper) handle(conn net.Conn) {
default:
if err := r.addFilter(msg); err != nil {
logger.Error("add filter", fieldError, err)
if _, err := conn.Write(ackResponse); err != nil {
if _, err = conn.Write(ackResponse); err != nil {
logger.Error("ack write", fieldError, err)
}
continue
Expand Down
12 changes: 6 additions & 6 deletions reaper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ func TestReapContainer(t *testing.T) {

t.Cleanup(func() {
// Ensure the container was / is removed.
err := cli.ContainerRemove(ctx, resp.ID, container.RemoveOptions{})
err = cli.ContainerRemove(ctx, resp.ID, container.RemoveOptions{})
require.Error(t, err)
require.True(t, errdefs.IsNotFound(err))
})
Expand Down Expand Up @@ -607,7 +607,7 @@ func TestReapNetwork(t *testing.T) {

t.Cleanup(func() {
// Ensure the network was / is removed.
err := cli.NetworkRemove(ctx, resp.ID)
err = cli.NetworkRemove(ctx, resp.ID)
require.Error(t, err)
require.True(t, errdefs.IsNotFound(err))
})
Expand All @@ -631,7 +631,7 @@ func TestReapVolume(t *testing.T) {

t.Cleanup(func() {
// Ensure the volume was / is removed.
err := cli.VolumeRemove(ctx, resp.Name, false)
err = cli.VolumeRemove(ctx, resp.Name, false)
require.Error(t, err)
require.True(t, errdefs.IsNotFound(err))
})
Expand Down Expand Up @@ -667,7 +667,7 @@ func TestReapImage(t *testing.T) {
return
}
var result types.BuildResult
err := json.Unmarshal(*msg.Aux, &result)
err = json.Unmarshal(*msg.Aux, &result)
require.NoError(t, err)
imageID = result.ID
}
Expand All @@ -677,8 +677,8 @@ func TestReapImage(t *testing.T) {

t.Cleanup(func() {
// Ensure the image was / is removed.
resp, err := cli.ImageRemove(ctx, imageID, image.RemoveOptions{})
require.Error(t, err)
resp, errc := cli.ImageRemove(ctx, imageID, image.RemoveOptions{})
require.Error(t, errc)
require.Empty(t, resp)
})

Expand Down

0 comments on commit 5d57bf3

Please sign in to comment.