-
Notifications
You must be signed in to change notification settings - Fork 852
.cirrus.yml: Test Vendoring bump golang #6386
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
.cirrus.yml: Test Vendoring bump golang #6386
Conversation
.cirrus.yml
Outdated
| # Runs within Cirrus's "community cluster" | ||
| container: | ||
| image: docker.io/library/golang:1.24.0 | ||
| image: docker.io/library/golang:1.24.7 |
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.
would it not be better to just use the 1.24 tag without patch release instead? That way it should always use the latest which means we don't have to update this manually so often.
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 value exactly matches what's in go.mod.
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.
right but we get bumps like #6385 where dependencies like to bump to go 1.24.2 for whatever reasons... so then it means it requires a manual update here.
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.
Why isn't the bot updating this part, too?
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.
well you could make the action do that, but I don't think you can teach renovate to do that which is what I would care about mainly?
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 will rebase this PR with version changed to 1.24 once @nalind agrees.
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 thought renovate couldn't handle the layout of the container-libs repository (it hasn't opened one for any of the components we get from it), and keeping the dependency updates automated was part of the motivation for podmanbot.
The do-not-merge bit isn't only being set because it's still being debugged?
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.
renovate cannot handle proper updates in the container-libs repo, for updates here it should work fine in theory. Though we still have to bump it to tagged versions once to make the go pseudo version work.
The point of podmanbot is to vendor every PR as WIP PR here and ensure all buildah tests still pass not to merge them
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.
@flouthoc Make a commit to vendor latest main from go.podman.io/{common,image,storage} and then also make a commit to bump this version here 1.24.2 then it will make all the test PRs work.
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.
@Luap99 there is no 1.24.2 on https://hub.docker.com/_/golang , I guess I will just do 1.24
Edit: Nevermind 1.24.2 is there.
62e4ea1 to
233aee6
Compare
|
Ephemeral COPR build failed. @containers/packit-build please check. |
Signed-off-by: flouthoc <[email protected]>
Fixes issue caught in containers#6385 Signed-off-by: flouthoc <[email protected]>
233aee6 to
5bb6d9e
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.
LGTM
|
@nalind @TomSweeneyRedHat PTAL |
|
LGTM |
|
@nalind gentle reminder ! is there anything blocking this PR. |
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.
It'll do. Maybe the gaps are something agentic AI can fill.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flouthoc, Luap99, nalind 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 |
|
/lgtm |
Fixes issue caught in #6385
What type of PR is this?
What this PR does / why we need it:
How to verify it
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?