-
Notifications
You must be signed in to change notification settings - Fork 18k
x/website: adjust release notes to say that cmd/fix's buildtag fix only kicks in with go.mod containing "go 1.18" or later #51873
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
Comments
Change https://go.dev/cl/394675 mentions this issue: |
Friendly ping @rsc @ianlancetaylor or @dmitshur for confirmation that we should indeed backport this to 1.18.1 :) It seems pretty clear to me, but I also don't want to add issues to the milestone without any notice. |
Going ahead with marking this for 1.18.1 for visibility. Happy for the backport issue to be removed if my conclusions are wrong. @gopherbot, please backport to Go 1.18. This seems like a |
Backport issue(s) opened: #52011 (for 1.18). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases. |
Sorry for not paying attention to this. I think that we should instead just change the documentation. I think that waiting for another release was the right move here, given that people using 1.17 may still want to support older releases even though we don't support them ourselves. |
Hmm, OK. I think we then need to abandon the CL and close the backport issue, and close this issue with an x/website CL to tweak the 1.18 changelog. Does that sound right? |
SGTM. Thanks. |
Yep, that sounds right. Go 1.17 shouldn't have applied the fix because when 1.17 was released, 1.16 was still supported (and wouldn't build with the fix applied), and we don't want And arguably |
Closed the backport issue and retitled. I'll leave the x/website fix to someone else, as I'm not able to submit changes in that repo. |
For the record you should be able to send in changes to the repo, although (unfortunately) two other people with appropriate access need to approve. |
Change https://go.dev/cl/396879 mentions this issue: |
Fixes golang/go#51873 Change-Id: Ied51c36a30cd18ff4a84d164e62df2690931d94f Reviewed-on: https://go-review.googlesource.com/c/website/+/396879 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Trust: Ian Lance Taylor <iant@google.com>
https://go.dev/doc/go1.18 says:
And the original issue agrees:
#41184
https://go.googlesource.com/proposal/+/master/design/draft-gobuild.md
However, the cmd/fix code incorrectly requires
go 1.18
or later for the removal of// +build
lines to happen:I assume this was a mistake in the code. I only noticed because I have a module with build tags and
go 1.17
in itsgo.mod
, signaling that I no longer support Go 1.16.x, and yet I was surprised to see thatgo fix
wouldn't clean up the build tags until I pushedgo.mod
togo 1.18
.I have the rather trivial fix ready. The point of this issue is that, assuming I'm right, this would need to be backported for Go 1.18.x, to make the release notes actually reflect reality.
cc @rsc @ianlancetaylor
The text was updated successfully, but these errors were encountered: