-
Notifications
You must be signed in to change notification settings - Fork 65
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
chore: remove patch version from go version in go.mod #88
Conversation
Hello, technically speaking, the patch part of the version is now a kind of requirement, there is some tolerance for compatibility reasons. I don't remember where I saw the explanation, I will find it. |
🤔 I didn't find where I saw that, maybe I made a confusion with Why is this important for you? the |
It means downstream projects need to set same go version |
I guess since 1.22 correct? |
I dont see anything in the documentation that implies youre forced to use 1.21.0 and cant go to 1.21.1. Everything in the go mod docs indicate that this is the minimum go version required for this package and consumers are free to go to a later version as needed. Is there an error youre observing? |
I understand what you called "noise" (the useless patch version), I had the same reaction at the beginning.
As I didn't find a reference, I will not be able to really answer this question. But without an official reference, I will not fight against this change. |
Now I'm forced to set Nothing functional that's just a nit but still... I guess I will have the same issue in the future if someone is doing the same so not directly directed to you but go toolchain shenanigans 😅 |
I don't take it badly: I did it because I was thinking it was a good practice. I'm annoyed not to find the reference. |
I think I have a kind of reference:
This means that
The I know it's not really explicit, and I'm sure I read an explicit message about that, I'm annoyed to not find it. |
Ah thanks for this feedback, ok I get it better now. |
The PR moby/buildkit#5118 has been merged, is your PR still required? |
Not required, I guess we can't do anything about it if we have other dep doing the same and related to your previous comment I think this is fine now. Thanks for your time! |
@ldez I can add a bit of context on the why here because it's been a thorn for us (at work) for a while. The new automatic toolchain upgrades introduced in Go 1.21 mean that the As a library, though, the practical effect of this is that updating the Go version in our |
Agree. How would we feel about using 1.15 since that was the latest version that the travisci build supported? |
I think 1.19, or maybe 1.20, would be good since that shouldn't trigger any inadvertent/unwanted Go version changes downstream. |
I talk about that inside Gofrs Slack channel. The new MVS clarification of the meaning of the I updated the Go version to the minimally supported Go version by the Go team, I did it with the knowledge of the impact, and I think that is not a problem to require a Go version supported by the Go team. The problem exposed through the PR is the difference between "release family" and "version". |
I was thinking that setting it to 1.20 would be "safe" since everyone should be using at least 1.21 now, but that's definitely not a point I'm going to argue over.as long as we're not pushing consumers onto newer versions of Go. |
Going forward, though, we shouldn't change the version unless we are using newer language features |
I don't share this opinion because using unmaintained go1.19 or go1.20 is a security issue. But I'm against, for a lib, using the latest Go version (currently go1.22), or the latest patch version, unless it's really needed. (I think we agree on that) I will not update the |
The whole versioning has become so complex; fun fact: go now can downgrade language versions per package / file; despite the go1.0 compatibility promise, it may disable features. Another fun example; backward compatibility code (i.e. files with stubs for old go versions) may no longer always work; see opencontainers/runc#4277 (comment) |
I guess my point is that the version of Go being used won't be what's in this library's FWIW, I've personally set |
Philosophically, I share @dylan-bourque's opinion. I'd prefer that we avoid compelling users to upgrade go versions unless we are leveraging a specific feature of the go version in our mod file. I definitely get that people should be upgrading, but the reality is there are some cases where they don't or can't for a number of reasons and using our package shouldn't get in the way of that. Compelling people to upgrade (as we see in this very PR ;)) does have a side effect of increasing friction to using our package and we want to make the use of our package a smooth experience. That doesn't mean that we need to support old go versions. We can continue testing against stable and old stable, and guarantee that we fix bugs for latest go and the latest version of our package. I think though that we should not be the pressure that forces people to upgrade. |
Definitely 👍 👍👍👍 on |
I think we all agree on the background subject: don't update the Go version unless you needed. I just have no problem defining the old stable as a requirement (not day one, and not the latest patch). |
@thaJeztah we had someone run into that with |
I found the reference I was thinking of (yes I never give up 😄) Code QL, when using at least go1.21, reports a warning: This report is about this file.
The documentation referenced inside the screenshot is https://go.dev/doc/toolchain#version If you add my previous comment with this warning and this doc, we have the full overview of the topic. |
relates to #57 (comment)