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.

Sign up for GitHub

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

Jump to bottom

Add alias for podman network rm -> remove #8404

Merged
merged 1 commit into from
Nov 20, 2020
Merged

Add alias for podman network rm -> remove #8404

merged 1 commit into from
Nov 20, 2020

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Nov 18, 2020

docker network remove exists and is alias to docker network rm.

Bug for bug compatible.

Fixes: #8402

Signed-off-by: Daniel J Walsh [email protected]

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 18, 2020
Luap99
Luap99 reviewed Nov 18, 2020
View reviewed changes
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM

@jwhonce
Copy link
Member

jwhonce commented Nov 18, 2020

/hold
/lgtm

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 18, 2020
@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 18, 2020
@TomSweeneyRedHat
Copy link
Member

Probably ought to add at least a regression test. Perhaps man page or bash completions? I'm fine if we decide not to doc or complete though.

@zhangguanzhang
Copy link
Collaborator

LGTM

@vrothberg
Copy link
Member

Probably ought to add at least a regression test. Perhaps man page or bash completions? I'm fine if we decide not to doc or complete though.

I agree. We shouldn't merge without a test.

@Luap99
Copy link
Member

Luap99 commented Nov 19, 2020

@TomSweeneyRedHat FYI, shell completion for command and flag names must/can no longer be added manually. This is now done by default. There is also a test in place to ensure that the necessary completions are added (e.g command parameters) . Command and flag aliases are never completed.

@TomSweeneyRedHat
Copy link
Member

@Luap99 thanks for the reminder. I'd forgotten about that, yet another side benefit of your excellent contribution.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 19, 2020
@rhatdan
Add alias for podman network rm -> remove
f4eac94 
docker network remove exists and is alias to docker network rm.

Bug for bug compatible.

Signed-off-by: Daniel J Walsh <[email protected]>
@rhatdan
Copy link
Member Author

rhatdan commented Nov 19, 2020

Added tests

@TomSweeneyRedHat
Copy link
Member

LGTM

@mheon
Copy link
Member

mheon commented Nov 19, 2020

I want to wait until CI runs to drop a /lgtm - I'm a bit worried the test framework won't pick up the tests since they're not top-level It(...) calls.

@rhatdan
Copy link
Member Author

rhatdan commented Nov 19, 2020

@mheon They were tested locally.

@zhangguanzhang
Copy link
Collaborator

zhangguanzhang commented Nov 20, 2020 •
edited
Loading

ci errored
https://storage.googleapis.com/cirrus-ci-6707778565701632-fcae48/artifacts/containers/podman/5207530957701120/html/int-remote-ubuntu-20-root-host.log.html

         Running: podman-remote [options] pod stats --no-stream
[+2840s] Error: unable to obtain cgroup stats: open /sys/fs/cgroup/pids/pids.current: no such file or directory

on my host ,

[root@Centos8 ~]# ls -l /sys/fs/cgroup/pids/ | grep pids
# no output

#8397

@mheon
Copy link
Member

mheon commented Nov 20, 2020

Restarted jobs

vrothberg
vrothberg reviewed Nov 20, 2020
View reviewed changes
Copy link
Member

@vrothberg vrothberg 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-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 20, 2020
@zhangguanzhang
Copy link
Collaborator

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 20, 2020
@openshift-merge-robot openshift-merge-robot merged commit eb4b26a into containers:master Nov 20, 2020
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Luap99 Luap99 Luap99 left review comments

@vrothberg vrothberg vrothberg left review comments

Assignees

@jwhonce jwhonce

@vrothberg vrothberg

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error: unrecognized command podman network remove
9 participants
@rhatdan @openshift-ci-robot @jwhonce @TomSweeneyRedHat @zhangguanzhang @vrothberg @Luap99 @mheon @openshift-merge-robot