Skip to content
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

Vendor in latest containers/(storage, image) #1255

Merged
merged 1 commit into from
Dec 5, 2022

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Dec 5, 2022

Signed-off-by: Daniel J Walsh dwalsh@redhat.com

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@openshift-ci openshift-ci bot added the approved label Dec 5, 2022
@rhatdan
Copy link
Member Author

rhatdan commented Dec 5, 2022

@edsantiago PTAL

@rhatdan
Copy link
Member Author

rhatdan commented Dec 5, 2022

@mtrmac @vrothberg @flouthoc @mheon PTAL

@mheon
Copy link
Member

mheon commented Dec 5, 2022

Fixing the GetLockfile -> GetLockFile bit in Podman is going to be fun.

LGTM

@vrothberg
Copy link
Member

/hold

We need an accompanying PR against Podman. Otherwise, we cannot vendor-in c/common until that is fixed which will potentially block other work.

Copy link
Collaborator

@flouthoc flouthoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 5, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, rhatdan

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rhatdan
Copy link
Member Author

rhatdan commented Dec 5, 2022

@vrothberg I am working this through the entire process to get there. I plan on working this through into Buildah and then into Podman. I also have PR for containers/image opened.

@rhatdan
Copy link
Member Author

rhatdan commented Dec 5, 2022

BTW Nothing is breaking except for linters complaining about depracated interfaces.

@rhatdan
Copy link
Member Author

rhatdan commented Dec 5, 2022

PR Opened containers/podman#16744

@mtrmac
Copy link
Contributor

mtrmac commented Dec 5, 2022

Fixing the GetLockfile -> GetLockFile bit in Podman is going to be fun.

That exists in containers/podman#16581 ; if that is merged, then c/common can be vendored, or not, into Podman at any unrelated time.

(It’s also still an option to remove the deprecation annotation from c/storage’s lockfile.Locker; it’s an interface we can’t evolve, but existing users who are happy with the previous set of capabilities can continue to use it just fine, and only migrate to a the new non-interface if they need the newly added methods. I think it’s cleaner to deprecate it — primarily to inform future work in c/storage — but that’s a very weak opinion and the downsides in downstream consumers are apparent.)

@rhatdan
Copy link
Member Author

rhatdan commented Dec 5, 2022

Since storage was merged into Podman, I am merging this.

@openshift-merge-robot openshift-merge-robot merged commit 370a48d into containers:main Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants