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

Go: bump tested Go version to 1.19.5 #831

Merged
merged 3 commits into from
Feb 20, 2023
Merged

Go: bump tested Go version to 1.19.5 #831

merged 3 commits into from
Feb 20, 2023

Conversation

ansiwen
Copy link
Collaborator

@ansiwen ansiwen commented Feb 9, 2023

This bumps the Go version of the test container to 1.19.5, which is the oldest supported Go version as of now.

The go.mod file (including an upgrade of its dependencies) and the revive environment is bumped to 1.19 as well.

Signed-off-by: Sven Anderson sven@redhat.com

Closes: #825

@ansiwen ansiwen added the no-API This PR does not include any changes to the public API of a go-ceph package label Feb 9, 2023
@phlogistonjohn
Copy link
Collaborator

The gist of it looks good, but the tests are failing. Ping me once the test failures are resolved. Thanks!

@ansiwen ansiwen force-pushed the pr/ansiwen/go-update branch 4 times, most recently from 2e50feb to b962630 Compare February 9, 2023 17:23
@ansiwen
Copy link
Collaborator Author

ansiwen commented Feb 9, 2023

@phlogistonjohn apparently the new gofmt has it's own idea how comments have to look like. :-/

Also I changed the go.mod version to 1.19, because I found contradicting information. In go help mod tidy it says:

By default, tidy acts as if the -compat flag were set to the
version prior to the one indicated by the 'go' directive in the go.mod
file.

But in https://go.dev/ref/mod#go-mod-file-go it says:

For packages within the module, the compiler rejects use of language features introduced after the version specified by the go directive. For example, if a module has the directive go 1.12, its packages may not use numeric literals like 1_000_000, which were introduced in Go 1.13.

The second behavior seems more important to us.

@ansiwen
Copy link
Collaborator Author

ansiwen commented Feb 9, 2023

Also the other jobs are failing because of the VCS stamping. I will continue on that later.

@phlogistonjohn
Copy link
Collaborator

@phlogistonjohn apparently the new gofmt has it's own idea how comments have to look like. :-/

sigh OK, it looks very mechanical (mostly just increased indents) and so I'll trust the change w/o going over it with a fine toothed comb.

Also I changed the go.mod version to 1.19, because I found contradicting information. In go help mod tidy it says:

By default, tidy acts as if the -compat flag were set to the
version prior to the one indicated by the 'go' directive in the go.mod
file.

But in https://go.dev/ref/mod#go-mod-file-go it says:

For packages within the module, the compiler rejects use of language features introduced after the version specified by the go directive. For example, if a module has the directive go 1.12, its packages may not use numeric literals like 1_000_000, which were introduced in Go 1.13.

The second behavior seems more important to us.

Indeed! Good catch, thanks.

Also the other jobs are failing because of the VCS stamping. I will continue on that later.

OK, that's fine. This is new to me as well and I'll read up a bit on it too.

@phlogistonjohn
Copy link
Collaborator

golang/go#51723 turned up in my search. So it seems that because we build implements tool it kicks in but wouldn't for our runs of go test.

Sounds like we need the -buildvcs=auto flag sprinkled around?

@ansiwen
Copy link
Collaborator Author

ansiwen commented Feb 13, 2023

Yeah, let's finish this after tue release then.

This bumps the Go version of the test container to 1.19.5, which is
the oldest supported Go version as of now.

The go.mod file (including an upgrade of its dependencies) and the
revive environment is bumped to 1.19 as well.

Signed-off-by: Sven Anderson <sven@redhat.com>
Signed-off-by: Sven Anderson <sven@redhat.com>
@ansiwen ansiwen force-pushed the pr/ansiwen/go-update branch 2 times, most recently from e0c2924 to 0109490 Compare February 20, 2023 17:29
Signed-off-by: Sven Anderson <sven@redhat.com>
@ansiwen
Copy link
Collaborator Author

ansiwen commented Feb 20, 2023

Ok, finally CI is succeeding. I had to use GOFLAGS to disable the VCS stamps, there is not other environment variable. Oh, and auto is the default, which is obviously not working. So we need false.

@ansiwen
Copy link
Collaborator Author

ansiwen commented Feb 20, 2023

@phlogistonjohn PTAL, and better review the individual commits.

Copy link
Collaborator

@phlogistonjohn phlogistonjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@mergify mergify bot merged commit 50668e7 into master Feb 20, 2023
@mergify mergify bot deleted the pr/ansiwen/go-update branch February 20, 2023 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-API This PR does not include any changes to the public API of a go-ceph package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update CI runs for current go version(s)
2 participants