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

move code from Podman so it can be shared with conmon-rs #1083

Merged
merged 2 commits into from
Jul 5, 2022

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Jul 5, 2022

see the commit descriptions and containers/conmon-rs#504

Luap99 added 2 commits July 5, 2022 18:21
conmon-rs currently depends on podmans pkg/kubeutils. Since podman wants
to import conmon-rs go code this can cause many problems.

To fix this we move the shared code to c/common so both projects can use
it without trouble. Also rename the package to pkg/resize from kubeutils
since this contains really only one function and the name is much more
clear.

see containers/conmon-rs#504

Signed-off-by: Paul Holzinger <[email protected]>
conmon-rs currently depends on podmans utils.CopyDetachable(). Since podman
wants to import conmon-rs go code this can cause many problems.

To fix this we move the shared code to c/common so both projects can use
it without trouble.

see containers/conmon-rs#504

Signed-off-by: Paul Holzinger <[email protected]>
@openshift-ci openshift-ci bot added the approved label Jul 5, 2022
@Luap99
Copy link
Member Author

Luap99 commented Jul 5, 2022

@haircommander
Copy link
Contributor

LGTM, though, can you think of any reason not just to have them in conmonrs?

@haircommander
Copy link
Contributor

they could still be imported by podman (once conmon-rs is being vendored), and ideally all of these would be used strictly by the conmon-rs client in the future

@Luap99
Copy link
Member Author

Luap99 commented Jul 5, 2022

LGTM, though, can you think of any reason not just to have them in conmonrs?

That would also work but I personally prefer to have shared code in c/common.

@Luap99
Copy link
Member Author

Luap99 commented Jul 5, 2022

they could still be imported by podman (once conmon-rs is being vendored), and ideally all of these would be used strictly by the conmon-rs client in the future

CopyDetachable is also used on the podman-remote side (pkg/bindings)

@mheon
Copy link
Member

mheon commented Jul 5, 2022

LGTM

@mheon
Copy link
Member

mheon commented Jul 5, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Jul 5, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 5, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99, saschagrunert

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

@openshift-ci openshift-ci bot merged commit dd1c331 into containers:main Jul 5, 2022
@Luap99 Luap99 deleted the resize branch July 5, 2022 18:26
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.

4 participants