-
Notifications
You must be signed in to change notification settings - Fork 76
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
ci: bump Go versions #194
ci: bump Go versions #194
Conversation
e4fc8ee
to
2d0d946
Compare
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.
Thanks!
.github/workflows/test.yml
Outdated
run: | | ||
cd $HOME | ||
mkdir $HOME/gotip | ||
cd $HOME/gotip | ||
|
||
wget -O gotip.tar.gz https://go.googlesource.com/go/+archive/${GO_COMMIT}.tar.gz | ||
tar -xf gotip.tar.gz | ||
echo "devel go1.19-${GO_COMMIT}" >VERSION | ||
echo "devel go1.20-${GO_COMMIT}" >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.
note that master is 1.21 now :)
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.
Good point! In any case I've reverted this change as the tests fail with the latest commit from master
.
.github/workflows/test.yml
Outdated
- stable | ||
- oldstable |
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.
I'm personally not a fan of this, because in my opinion we want to be conscious about what major versions of Go we support.
We do tend to do the last two versions, but when a new version comes out, it's a human process to update the versions in multiple places (CI, go.mod, etc) and test that everything still works. Sometimes a few tests break, especially since we have tight coupling with some packages like testing - and we also don't want CI to start failing when a new version of Go is out :)
but perhaps @rogpeppe disagrees. Some people prefer their CI to track "latest stable" with the Go version, and then they can react when CI starts failing on its own. I personally prefer to keep CI closer to determinism, so that it's not likely to start failing on its own.
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.
Worth noting that depending on the project I also track versions of the operating systems manually. For example, on a project of mine, macos-latest
broke a test when that jumped from macos-11
to macos-12
. But, for most Go projects, OS upgrades don't change behavior.
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.
Ack, I've changed the PR to use Go versions 1.18.x
and 1.19.x
explicitly.
e47f330
to
dd94cda
Compare
.github/workflows/test.yml
Outdated
- name: Test | ||
run: | | ||
go test ./... | ||
go test -race ./... | ||
|
||
- name: Tidy | ||
if: matrix.os == 'ubuntu-latest' # no need to do this everywhere | ||
if: matrix.os == 'ubuntu-latest' && matrix.go-version == 'stable' # no need to do this everywhere |
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.
presumably this is wrong as it still mentions "stable"?
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.
/facepalm Good catch, thanks.
dd94cda
to
f8a3c0f
Compare
f8a3c0f
to
ee726b3
Compare
I noticed CI was still using Go 1.17 and 1.18.