-
Notifications
You must be signed in to change notification settings - Fork 20
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
Bump Go version to 1.22 #174
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for the code/linter/copyright changes. But if we still support 1.20 (i.e. we want all examples to be runnable using Go 1.20, per what's in go.mod
), why bump CI all the way up to 1.22?
.github/workflows/ci.yaml
Outdated
@@ -25,7 +25,7 @@ jobs: | |||
- name: setup-go | |||
uses: actions/setup-go@v5 | |||
with: | |||
go-version: 1.21.x | |||
go-version: 1.22.x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If go.mod
states Go 1.20, and we're not using a matrix here to run these steps across multiple versions, seems like we should actually set this to 1.20.x
instead of updating it to 1.22. Why should this be using latest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was following current, suppose its better to test old versions of go then latest. Have set at 1.20.x.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted, lint breaks and is sensitive to go version. We could create a matrix for these test cases but I think it's safe for example code to be run at the latest version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
safe for example code to be run at the latest version
Then we should change go.mod
to 1.22, or else we're not actually verifying that the repo works, like for people that pull down the contents and use an earlier (ostensibly) supported version.
I think it's better to just leave this at the older version. Or we could move it to Go 1.21, since that is the oldest Go version still getting patches.
lint breaks and is sensitive to go version
Shouldn't it be sensitive to the version of golangci-lint, not the version of go? Or if there's a breakage due to gofmt changes from 1.20 to 1.22, we can just format it with 1.20 for now.
Bumps go version to 1.22 with the new min at 1.20. Also add LICENSE file updates along with a bump to buf and golanglint-ci. Fixes from lintci. Please see commit-by-commit.