fix up a number of misplace commands #2471
Conversation
* ps now on main command * sign is no longer on main commmand * ls, list no longer are valid main aliases for images * ls, list does work for podman image Signed-off-by: baude <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: baude 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 |
openshift-ci-robot
added
the
approved
baude
requested review from
edsantiago
and removed request for
mrunalp and
TomSweeneyRedHat
| @@ -158,10 +158,9 @@ var ( | |||
| psCommand cliconfig.PsValues | |||
| psDescription = "Prints out information about the containers" | |||
| _psCommand = &cobra.Command{ | |||
| Use: "list", | |||
| Aliases: []string{"ls", "ps"}, | |||
There was a problem hiding this comment.
Did you mean to remove 'ls' and 'list' aliases?
| @@ -87,7 +87,7 @@ var ( | |||
| imagesCommand cliconfig.ImagesValues | |||
| imagesDescription = "lists locally stored images." | |||
|
|
|||
| _imagesCommand = &cobra.Command{ | |||
| _imagesCommand = cobra.Command{ | |||
There was a problem hiding this comment.
This surprised me: everywhere else, the convention is
_xxxCommand = &cobra.Command{
It's even more jarring because of the corresponding change in main.go.
There was a problem hiding this comment.
it is the black magic to get the subcommand image to honor images, ls, list but not honor ls, list on the main podman menu. Perhaps better put:
- podman image ls should work
- podman ls should not work
| @@ -14,14 +14,14 @@ var ( | |||
| Long: imageDescription, | |||
| }, | |||
| } | |||
| _imagesSubCommand = _imagesCommand | |||
There was a problem hiding this comment.
Does this actually copy it? I'm wondering how we get two images commands with different aliases out of a simple equals
There was a problem hiding this comment.
Alright, so we're doing actual values here, instead of pointers, and that forces the copy. Ok...
There was a problem hiding this comment.
we do not ... at least when the original was defined as a pointer. this was the only way it actually worked.
There was a problem hiding this comment.
Yeah, it looks like Go automagically copies when it's a struct assigned by value, not reference.
There was a problem hiding this comment.
Can we have a comment here like "Don't use pointer to force copy" so someone doesn't see this in a month and say "why is this different? I'm going to make this look like the others"
|
Code tests well. However ubuntu test isn't happy atm. |
|
This is a partial fix, now the podman commands match the man pages. But we still have issues with Lets merge this and then work on those. /lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
Signed-off-by: baude [email protected]