Rename "list" and "delete" to "containers" and "rm"; add "images" and "rmi" commands #32
Conversation
cmd/buildah/main.go
Outdated
| return nil | ||
| } | ||
| app.Commands = []cli.Command{ | ||
| { |
There was a problem hiding this comment.
Thought this was going to be list?
There was a problem hiding this comment.
We're already using "list" for listing containers that buildah is working with, though I could be swayed to "list-images" to parallel with "delete-image".
There was a problem hiding this comment.
My bad, ignore this. I'd forgotten list was for containers. Images is good.
cmd/buildah/images.go
Outdated
| Usage: "omit column headings", | ||
| }, | ||
| cli.BoolFlag{ | ||
| Name: "qq", |
There was a problem hiding this comment.
Depending on the online context, qq means 'crying eyes' or 'just quit the game as you've no skills'. Perhaps --onlyIDs or --short or --min?
There was a problem hiding this comment.
Oh. I was going for something along the lines of 'docker images -q', but am not opposed to renaming the flag. Any preference?
There was a problem hiding this comment.
At first I thought just --q would be good, but then remembered docker images uses --quiet or -q to only show the ids. Can I vote to change the flags to --omit for the column headings and --quiet for the image ids only?
There was a problem hiding this comment.
The --quiet flag is what we already for headings in "list", that'll need to be changed there to keep them consistent. Updating.
TomSweeneyRedHat
left a comment
TomSweeneyRedHat
left a comment
There was a problem hiding this comment.
LGTM, couple nits for consideration
nalind
changed the title
|
I would prefer not to add additional verbs, just use buildah list and buildah delete. |
|
LGTM. For images docker uses 'images' for list and 'rmi' to remove. For containers 'ps' and 'rm'. Should we emulate that or do you think the code should recognize the object and do the right thing? |
nalind
changed the title
|
We can overload delete so that it looks for a container first, but with free-form names around, we're inevitably going to need some way to specify what type of thing was intended. We have distinctly different things to display for images and containers, and every output format I can come up with that encompasses both just looks messier if you only care about one or the other. |
|
Can't we just force user to specify --image or --container when their is conflict. |
|
☔ The latest upstream changes (presumably b58b778) made this pull request unmergeable. Please resolve the merge conflicts. |
Rename the "list" and "delete" commands to "containers" and "rm", respectively, and add "images" and "rmi" counterparts for them. Signed-off-by: Nalin Dahyabhai <[email protected]>
nalind
changed the title
nalind
changed the title
|
☀️ Test successful - status-redhatci |
4 participants
Add some minimal image management commands. Neither is really in scope for a building tool, but it reduces the number of use cases where we need additional tools.