downgrade Go version from 1.26.0 to 1.25.8#756
Conversation
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Go version is hard-coded in multiple places (three workflows, Dockerfiles, Makefile, go.mod); consider centralizing it via a reusable workflow input or a shared build arg/variable to avoid divergence on the next version change.
- The Dockerfile and CLI Dockerfile use
ARG GO_VERSION=1.25while the Makefile/go.mod use1.25.8; if the intent is to pin to a specific patch version everywhere, consider aligning these to avoid subtle differences between local builds and container builds.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Go version is hard-coded in multiple places (three workflows, Dockerfiles, Makefile, go.mod); consider centralizing it via a reusable workflow input or a shared build arg/variable to avoid divergence on the next version change.
- The Dockerfile and CLI Dockerfile use `ARG GO_VERSION=1.25` while the Makefile/go.mod use `1.25.8`; if the intent is to pin to a specific patch version everywhere, consider aligning these to avoid subtle differences between local builds and container builds.
## Individual Comments
### Comment 1
<location path="go.mod" line_range="3" />
<code_context>
module github.com/docker/model-runner
-go 1.26.0
+go 1.25.8
require (
</code_context>
<issue_to_address>
**issue (bug_risk):** The `go` directive should use only major.minor (e.g., `go 1.25`) rather than a full patch version.
Using a patch version in `go.mod` will cause `go` tooling errors (e.g., `invalid go version`). Please change this to `go 1.25` and keep the specific patch level (1.25.8) enforced via CI/Docker or other tooling instead.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| module github.com/docker/model-runner | ||
|
|
||
| go 1.26.0 | ||
| go 1.25.8 |
There was a problem hiding this comment.
issue (bug_risk): The go directive should use only major.minor (e.g., go 1.25) rather than a full patch version.
Using a patch version in go.mod will cause go tooling errors (e.g., invalid go version). Please change this to go 1.25 and keep the specific patch level (1.25.8) enforced via CI/Docker or other tooling instead.
There was a problem hiding this comment.
Code Review
This pull request aims to downgrade the Go version. However, both the original version (1.26.0) and the new version (1.25.8) are not existing Go releases. This is a critical issue that will cause build failures for anyone using standard Go toolchains or Docker, as the specified Go versions and corresponding Docker images do not exist. I've added comments to all affected files pointing out this issue. Please correct the Go version to a valid, released version across all files.
doringeman
left a comment
There was a problem hiding this comment.
LGTM!
$ ./scripts/sync-go-version.sh check
Go version from go.mod: 1.25.8
Checking Go version consistency...
All files are in sync.
|
Was the problem that |
|
Thanks! Yes, we downgraded so we can import it in projects that still use 1.25. I'm totally fine with using 1.25, too. |
required to bump DMR in DD to latest version