-
Notifications
You must be signed in to change notification settings - Fork 663
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
Document Go version policy #1225
base: main
Are you sure you want to change the base?
Conversation
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.
lgtm
... we don't want to fall too behind; cve fixes etc!
CVE fixes would apply to projects using the spec, rather than the spec itself. I don't expect we'll see Go CVEs in the implementation of structs. |
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.
LGTM -- because we're a library, us staying conservative allows consumers to make the tradeoffs they need to (https://research.swtch.com/vgo-mvs) without being forced by us to change, and I think this policy is reasonably conservative even if we do have a compelling technical reason in the future to bump beyond 1.18 as a minimum. 👍
go.mod
Outdated
// The minimum Go release is only incremented when required by a feature. | ||
// At least 3 Go releases will be supported by the spec. | ||
// For example, updating this version to 1.19 first requires Go 1.21 to be released. | ||
// Update actions in .github/workflows when changing this version. |
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.
I bet we could get clever with somehow swapping from go-version:
to go-version-file:
for that leg of our GHA matrix to avoid this 🤔
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.
Turns out it was really easy, and actions/setup-go
has stable
and oldstable
aliases that match our intent there too, so we'd not need to bump them again: tianon/go-progress@1e3b16c 😂
(This is orthogonal though, so I can make a separate PR; Edit: done: #1229)
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.
Not completely orthogonal. With that change to GHA, we no longer needed this line.
Well, we don't export structs alone separately, and this project has a go.mod |
As described in https://research.swtch.com/vgo-mvs, because we're just a library (no binaries / "main" packages here), our |
Signed-off-by: Brandon Mitchell <[email protected]>
a158387
to
4dcf962
Compare
Fixes #899.