Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade golangci-lint GitHub Action go v5 and linter to 1.56 #223

Merged
merged 1 commit into from
Feb 24, 2024

Conversation

brandur
Copy link
Contributor

@brandur brandur commented Feb 23, 2024

A small one to upgrade:

  • golangci-lint GitHub Action from v4 to v5
  • golangci-lint from 1.55 to 1.56

A small change, but I upgraded locally at some point, and it's causing a
build issue in #212. The tparallel lint previously had a bug where
it'd want you to put a t.Parallel() on test cases that use t.Setenv(),
even though use of t.Parallel() with t.Setenv() is disallowed by Go's
test framework, so you'd have to mark those test cases with nolint.

That bug's been fixed now, and now the linter detects that the nolint
is no longer necessary, and automatically strips it out with my upgraded
golangci-lint version, causing a failure when I push to CI.

The v5 GitHub Action is also somewhat nice because it stops using a
deprecated NodeJS version, so produces fewer warnings in the log.

@brandur brandur force-pushed the brandur-golangci-lint-v5 branch 2 times, most recently from 363f08b to 2ea003f Compare February 23, 2024 04:32
@brandur
Copy link
Contributor Author

brandur commented Feb 23, 2024

@bgentry Super small one, and remember to upgrade your local version.

A small one to upgrade:

* golangci-lint GitHub Action from `v4` to `v5`
* golangci-lint from 1.55 to 1.56

A small change, but I upgraded locally at some point, and it's causing a
build issue in #212. The `tparallel` lint previously had a bug where
it'd want you to put a `t.Parallel()` on test cases that use `t.Setenv()`,
even though use of `t.Parallel()` with `t.Setenv()` is disallowed by Go's
test framework, so you'd have to mark those test cases with `nolint`.

That bug's been fixed now, and now the linter detects that the `nolint`
is no longer necessary, and automatically strips it out with my upgraded
golangci-lint version, causing a failure when I push to CI.

The `v5` GitHub Action is also somewhat nice because it stops using a
deprecated NodeJS version, so produces fewer warnings in the log.
@bgentry bgentry merged commit 48ef9ef into master Feb 24, 2024
10 checks passed
@bgentry bgentry deleted the brandur-golangci-lint-v5 branch February 24, 2024 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants