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

github/workflows: migrate CI to Go 1.19 and fix linting issues #2426

Merged
merged 19 commits into from
Nov 14, 2022

Conversation

fsouza
Copy link
Contributor

@fsouza fsouza commented Oct 20, 2022

Description

We want to make sure we can safely migrate to 1.19, so let's run it in CI. This PR ended-up being a bit big because it does multiple things (see individual commits for more details):

  1. upgrade golangci-lint to 1.50.0 (latest version) and fix violations
  2. fix GHA actions (set-output is now deprecated)
  3. Update GHA workflows to run Go 1.19, and add go version to all cache keys to make future upgrades easier (note: setup-go has built-in cache support, we probably don't need to use actions/cache directly, but I didn't want to change the config too much)
  4. run gofmt with Go 1.19 to reduce future diffs as folks upgrade to Go 1.19
  5. regen protos and configure the api job to use Go 1.19
  6. fix golangci-lint issues
    1. golangci-lint run --fix for violations that can be fixed automatically
    2. some search and replace for the ioutil deprecation
    3. two manual fixes (one nolint for a false positive and one code change where we pass ReadHeaderTimeout to http.Server)
    4. remove deprecated linters from .golangci.yml

I'd be happy to split it apart. Each of the items above could be a separate PR, but to be honest since many of the changes are either automatic or simple search and replace, I wonder if that's really necessary? Let me know either way and I'll adjust.

One note is that clutch is already distributed with Go 1.19 via the Docker image, but I'm unsure about who are the consumers of that.

Testing Performed

GitHub will test it for us :)

@fsouza fsouza requested a review from a team as a code owner October 20, 2022 02:56
@fsouza fsouza changed the title github/workflows: enable Go 1.19 in CI github/workflows: enable Go 1.19 in CI and fix linting issues Oct 20, 2022
tools/golangci-lint.sh Outdated Show resolved Hide resolved
tools/golangci-lint.sh Outdated Show resolved Hide resolved
@fsouza fsouza changed the title github/workflows: enable Go 1.19 in CI and fix linting issues github/workflows: migrate CI to Go 1.19 and fix linting issues Oct 21, 2022
@fsouza fsouza force-pushed the go-1.19-in-ci branch 4 times, most recently from 3024558 to 1f5a1cc Compare October 27, 2022 15:14
@github-actions
Copy link

github-actions bot commented Nov 3, 2022

This PR has been marked as stale after 7 or more days of inactivity. Please have a maintainer add the on hold label if this PR should remain open. If there is no further activity or the on hold label is not added, this PR will be closed in 3 days.

@github-actions github-actions bot added the stale Issue hasn't had activity in awhile label Nov 3, 2022
@fsouza fsouza removed the stale Issue hasn't had activity in awhile label Nov 4, 2022
backend/retry/retry.go Outdated Show resolved Hide resolved
fsouza added 10 commits November 7, 2022 16:35
To avoid issues, golangci-lint should be compile with the same version
of the Go.
We want to make sure we can safely migrate to 1.19, so let's run it in
CI.
gofmt has changed comments quite a bit.

Also use `go fix` to get rid of the legacy build tags.
Unfortunately, the output has changed from Go 1.17 to Go 1.19, so that
step can only run with one of the two versions. Let's prefer the most
recent one.
Removed deprecated linters, and also remove goimports (gci is a superset
of goimports).
It was deprecated in Go 1.16.
@danielhochman danielhochman merged commit 9aa9e74 into main Nov 14, 2022
@danielhochman danielhochman deleted the go-1.19-in-ci branch November 14, 2022 21:29
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.

5 participants