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

Bump Go version to 1.22 #174

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

emcfarlane
Copy link

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.

@emcfarlane emcfarlane marked this pull request as draft February 22, 2024 18:20
@emcfarlane emcfarlane marked this pull request as ready for review March 27, 2024 14:49
Copy link
Member

@jhump jhump left a 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?

@@ -25,7 +25,7 @@ jobs:
- name: setup-go
uses: actions/setup-go@v5
with:
go-version: 1.21.x
go-version: 1.22.x
Copy link
Member

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?

Copy link
Author

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.

Copy link
Author

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.

Copy link
Member

@jhump jhump Apr 18, 2024

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.

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