-
Notifications
You must be signed in to change notification settings - Fork 36
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
Revert to Go 1.18 in go.mod and make policy around this official #390
Conversation
@jacobweinstock based on the Go release policy, we'd want to be on a release that is not at its EOL? |
Note that is the minimum version supported. By setting it to 1.18, we don't prevent people from using latest version. I see several options here: we could stay on the smallest go version of dependencies, or follow the base go version available for some platforms (e.g. min of go version available in oldest supported debian/redhat/ubuntu) |
@@ -88,7 +88,6 @@ golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= | |||
golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o= | |||
golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= | |||
golang.org/x/term v0.19.0 h1:+ThwsDv+tYfnJFhF4L8jITxu1tdTWRTZpdsWgEgjL6Q= | |||
golang.org/x/term v0.19.0/go.mod h1:2CuTdWZ7KHSQwUzKva0cbMg6q2DMI3Mmxp+gKJbskEk= |
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.
Is this an effect of the downgrading?
I agree with the OP: libraries should honor the definition of the
I think the only reason to care about supported platforms is in implementation detail because you wouldn't want to use a language feature > than the oldest supported platform. Assuming there aren't any language feature issues I would stick with the definition of the |
Those are fair points, thanks for raising them. If we can get a rebase on main here then this can be merged. |
We don't seem to have an offical strategy for the Go version in go.mod. I believe that, as a library, we should only bump this version if there are dependencies we use that would require us to move to this version. I don't believe that we have any dependencies that warrant a bump. Signed-off-by: Jacob Weinstock <[email protected]>
This makes official our philosophy and policy for the Go version in go.mod. Signed-off-by: Jacob Weinstock <[email protected]>
What does this PR implement/change/remove?
We don't seem to have an official strategy for the Go version in go.mod. I believe that, as a library, we should only bump this version if there are dependencies we use that would require us to move to this version (ref). I don't believe that we have any dependencies that warrant a bump. Bumping this version will require user to update to this version. While it is a good practice to build with the latest version of Go, this requirement will be making a decision for users that should be theirs and not ours. We are a library not a binary to be built. In this aspect, we are using the version in go.mod incorrectly, in my opinion, by doing this. This assumes we don't have a known use case where bumping this version if needed. I don't know of any at the moment. If there are any we should bring them up.
Apologies, i know i approved the PR to update this. The affects of this didn't dawn on me then.
The other side of the coin
There can be reasons to bump this version. I think before we bump the version we should have use cases the bump and more carefully consider the affects. From the official Go docs:
"At go 1.21 or higher:
The go line declares a required minimum version of Go to use with this module.
The go line must be greater than or equal to the go line of all dependencies.
The go command no longer attempts to maintain compatibility with the previous older version of Go.
The go command is more careful about keeping checksums of go.mod files in the go.sum file." - ref
Checklist
The HW vendor this change applies to (if applicable)
The HW model number, product name this change applies to (if applicable)
The BMC firmware and/or BIOS versions that this change applies to (if applicable)
What version of tooling - vendor specific or opensource does this change depend on (if applicable)
Description for changelog/release notes