-
Notifications
You must be signed in to change notification settings - Fork 106
go.mod: use go 1.23 #1048
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.mod: use go 1.23 #1048
Conversation
We don't want our packages to force a Go version bump upstream. This PR changes our modules to just use `go 1.23` with no patch version.
Downstream issue: open-telemetry/opentelemetry-collector-contrib#40436 |
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.
Shouldn't the CI be updated as well to match this ?
Or are we still ok to run tests explicitly against 1.23.8 ?
Yes that's fine. Our modules are just declaring a compatibility, not saying that is has to be used only with this exact version of Go. |
We should run tests explicitly against a patch version to satisfy the go vuln checker |
detectors/gcp/go.mod
Outdated
go 1.23.8 | ||
go 1.23.0 | ||
|
||
toolchain go1.24.2 |
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.
Would this mean that at least go1.24.2 is required to build and compile this project? I'm ok with this, but is this intentional ?
Edit: Saw the failing lint - was this added to address the failed lint check ?
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.
toolchain
is something that Go adds automatically now when you run go mod tidy
. go1.24.2
was the local toolchain I was using. I've tried setting an env var in CI to ignore it.
@braydonk can you fix the lint so we can get this merged? This is impacting other projects, so we should get a patch release out with this soon |
I am unsure how to fix the lint step. |
You probably need to update golangci-lint here:
|
I've solved the lint problem. I'm not sure how to handle that final e2e test failure. |
From the logs, I see
Following the link to the build logs, I see:
If you want to update the toolchain version, you probably need to bump this go version here, otherwise GAE won't build:
I'm surprised this doesn't also need an update, but the GAE flex is passing:
|
Since the |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1048 +/- ##
==========================================
- Coverage 61.03% 0 -61.04%
==========================================
Files 56 0 -56
Lines 5903 0 -5903
==========================================
- Hits 3603 0 -3603
+ Misses 2143 0 -2143
+ Partials 157 0 -157 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
We don't want our packages to force a Go version bump downstream. This PR changes our modules to just use
go 1.23
with no patch version.