-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add podman manifest rm --ignore #24678
base: main
Are you sure you want to change the base?
Conversation
[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 |
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.
thank you!
Can you add/enhance a test in |
@@ -9,6 +9,12 @@ podman\-manifest\-rm - Remove manifest list or image index from local storage | |||
## DESCRIPTION |
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.
Should we be committing this if there is a corresponding .in
file?
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.
yes if we use .in the normal file should not be committed and added to the gitignore
Technically speaking there is no reason to use the .in file as it doesn't make use of the imports
cmd/podman/manifest/rm.go
Outdated
"github.com/containers/podman/v5/pkg/errorhandling" | ||
"github.com/spf13/cobra" | ||
) | ||
|
||
var ( | ||
rmCmd = &cobra.Command{ | ||
rmOptions = entities.ImageRemoveOptions{} | ||
rmCmd = &cobra.Command{ | ||
Use: "rm LIST [LIST...]", |
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.
need to add [options]
between rm and LIST
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.
Good catch.
@@ -9,6 +9,12 @@ podman\-manifest\-rm - Remove manifest list or image index from local storage | |||
## DESCRIPTION |
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.
yes if we use .in the normal file should not be committed and added to the gitignore
Technically speaking there is no reason to use the .in file as it doesn't make use of the imports
podman\-manifest\-rm - Remove manifest list or image index from local storage | ||
|
||
## SYNOPSIS | ||
**podman manifest rm** *list-or-index* [...] |
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.
Also must add [*options*]
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.
Nice. the .in file should not have been checked in.
When removing manifests, users should be allowed to ignore ones that no longer exists. Signed-off-by: Daniel J Walsh <[email protected]>
When removing manifests, users should be allowed to ignore ones that no longer exists.
Does this PR introduce a user-facing change?