Skip to content

Commit

Permalink
Upgrade golangci-lint from 1.56 to 1.59 (#466)
Browse files Browse the repository at this point in the history
We're now a few versions behind the latest golangci-lint, and given that
it moves pretty fast and upgrading too many versions at once can be a
bit of a job, here, bump us up to version 1.59 to get us back latest.

Fix a couple breakages, but relatively few of them. A couple linters
that we had disabled were renamed, so fix those in configuration to
continue to disable them because they continue to be awful (`err113` and
`mnd`).

There are a couple useful linters brought in for Go 1.22:

* `copyloopvar`: Encourages code to _not_ capture variables in loops
  (e.g. `for ...; i := ; ...`) because as of 1.22, it's no longer
  necessary.

* `intrange`: Encourages looping by integer to be written with `range`
  like `for i := range(5)` instead of the C-style `for i := 0; i < 5; i++`.

I like both of these, but we're still technically trying to support Go
1.21, so I've disabled them for the time being. Once we drop 1.21 (which
could plausibly be as close as a few weeks away as 1.23 is expected
soon), we can turn them back on.
  • Loading branch information
brandur authored Jul 24, 2024
1 parent 3a4a06d commit 2ae1b6c
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 11 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ jobs:
name: lint
runs-on: ubuntu-latest
env:
GOLANGCI_LINT_VERSION: v1.56
GOLANGCI_LINT_VERSION: v1.59
permissions:
contents: read
# allow read access to pull request. Use with `only-new-issues` option.
Expand Down
9 changes: 7 additions & 2 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ linters:
- unused

disable:
# disabled, but which we should enable after dropping support for Go 1.21 as
# they're kind of a good idea
- copyloopvar # lints to make sure that loop variables are _not_ captured since it's not required in Go 1.22+
- intrange # encourages for loops to range over integers like `for i := range(5)` instead of a C-style for

# disabled, but which we should enable with discussion
- wrapcheck # checks that errors are wrapped; currently not done anywhere

Expand All @@ -23,8 +28,8 @@ linters:
# disabled because they're annoying/bad
- interfacebloat # we do in fact want >10 methods on the Adapter interface or wherever we see fit.
- godox # bans TODO statements; total non-starter at the moment
- goerr113 # wants all errors to be defined as variables at the package level; quite obnoxious
- gomnd # detects "magic numbers", which it defines as any number; annoying
- err113 # wants all errors to be defined as variables at the package level; quite obnoxious
- mnd # detects "magic numbers", which it defines as any number; annoying
- ireturn # bans returning interfaces; questionable as is, but also buggy as hell; very, very annoying
- lll # restricts maximum line length; annoying
- nlreturn # requires a blank line before returns; annoying
Expand Down
4 changes: 2 additions & 2 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1588,7 +1588,7 @@ func Test_Client_InsertMany(t *testing.T) {

jobs, err := client.driver.GetExecutor().JobGetByKindMany(ctx, []string{(noOpArgs{}).Kind()})
require.NoError(t, err)
require.Len(t, jobs, 2, "Expected to find exactly two jobs of kind: "+(noOpArgs{}).Kind()) //nolint:goconst
require.Len(t, jobs, 2, "Expected to find exactly two jobs of kind: "+(noOpArgs{}).Kind())
})

t.Run("TriggersImmediateWork", func(t *testing.T) {
Expand Down Expand Up @@ -4944,7 +4944,7 @@ func TestDefaultClientID(t *testing.T) {

startedAt := time.Date(2024, time.March, 7, 4, 39, 12, 123456789, time.UTC)

require.Equal(t, strings.ReplaceAll(host, ".", "_")+"_2024_03_07T04_39_12_123456", defaultClientID(startedAt)) //nolint:goconst
require.Equal(t, strings.ReplaceAll(host, ".", "_")+"_2024_03_07T04_39_12_123456", defaultClientID(startedAt))
}

func TestDefaultClientIDWithHost(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions internal/jobcompleter/job_completer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -592,12 +592,12 @@ func testCompleter[TCompleter JobCompleter](
// Signal to stop insertion and wait for the goroutine to return.
numInserted := stopInsertion()

require.Greater(t, numInserted, 0)
require.Positive(t, numInserted)

numCompleted, err := bundle.exec.JobCountByState(ctx, rivertype.JobStateCompleted)
require.NoError(t, err)
t.Logf("Counted %d jobs as completed", numCompleted)
require.Greater(t, numCompleted, 0)
require.Positive(t, numCompleted)
})

t.Run("SlowerContinuousCompletion", func(t *testing.T) {
Expand All @@ -617,12 +617,12 @@ func testCompleter[TCompleter JobCompleter](
// Signal to stop insertion and wait for the goroutine to return.
numInserted := stopInsertion()

require.Greater(t, numInserted, 0)
require.Positive(t, numInserted)

numCompleted, err := bundle.exec.JobCountByState(ctx, rivertype.JobStateCompleted)
require.NoError(t, err)
t.Logf("Counted %d jobs as completed", numCompleted)
require.Greater(t, numCompleted, 0)
require.Positive(t, numCompleted)
})

t.Run("AllJobStates", func(t *testing.T) {
Expand Down
2 changes: 0 additions & 2 deletions internal/riverinternaltest/riverdrivertest/riverdrivertest.go
Original file line number Diff line number Diff line change
Expand Up @@ -1022,8 +1022,6 @@ func Exercise[TTx any](ctx context.Context, t *testing.T,
t.Run("JobFinalizedAtConstraint", func(t *testing.T) {
t.Parallel()

ctx := context.Background()

capitalizeJobState := func(state rivertype.JobState) string {
return cases.Title(language.English, cases.NoLower).String(string(state))
}
Expand Down

0 comments on commit 2ae1b6c

Please sign in to comment.