-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[CI:BUILD] Add golang 1.21 update warning #22073
[CI:BUILD] Add golang 1.21 update warning #22073
Conversation
4b295e0
to
f5a4b1f
Compare
Cockpit tests failed for commit 4b295e0ab49ffe649cda4f6a5ffe79863a6669fc. @martinpitt, @jelly, @mvollmer please check. |
f5a4b1f
to
d10cbe8
Compare
@nalind @rhatdan @mtrmac @Luap99 PTAL. If this language is acceptable, I'd like to dupe this change into Buildah and Skopeo as well as any other golang repos you think important. See also containers/automation#187 |
Cockpit tests failed for commit f5a4b1fa9182ab5f188cb4df9226cd95d214e71e. @martinpitt, @jelly, @mvollmer please check. |
Cockpit tests failed for commit d10cbe89fbbc3ba0e6e6380f089bc5a4b5d8ae2d. @martinpitt, @jelly, @mvollmer please check. |
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 do like the idea of the warning.
It might be a bit clearer to explicitly separate the two:
go
is set to the latestgo
of all dependencies. That can’t be prevented, other than not updating the dependency.toolchain
is bumped when increasinggo
for some reason (and based on the currently running tools, not on what any dependency declares). If, at the time of thego
bump (or later), we choose atoolchain
version acceptable to us, it is not going to be auto-updated until ago
auto-update happens.
This is needed on the off-chance that some tool or a human suggests updating the minimum version to 1.21 or later. Since doing so would cause Fedora and Debian to start behaving differently WRT builds. Signed-off-by: Chris Evich <[email protected]>
d10cbe8
to
48b8d7f
Compare
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.
This LGTM
Meanwhile, https://bodhi.fedoraproject.org/updates/?packages=golang suggests that F38 now does support Go 1.21. So maybe we can update soon — but that is not a reason to not merge this PR right now.
/lgtm |
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.
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashley-cui, cevich The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This is needed on the off-chance that some tool or a human suggests updating the minimum version to 1.21 or later. Since doing so would cause Fedora and Debian to start behaving differently WRT builds.
Ref: containers/automation#187
Does this PR introduce a user-facing change?