-
Notifications
You must be signed in to change notification settings - Fork 874
Add support for images as well as containers to buildah list & delete #47
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
Conversation
|
Alternative to #32 |
cmd/buildah/delete.go
Outdated
| return fmt.Errorf("error reading build container %q: %v", name, err) | ||
| /* Maybe this was an image attempt to delete it */ | ||
| _, err = store.DeleteImage(name, true) | ||
| return fmt.Errorf("error deleting image or container container %q: %v", name, err) |
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.
"container container"?
This patch set will cleanup the output of buildah list adding truncate flags as well as support images.
| if len(images) > 0 && !quiet { | ||
| fmt.Printf("\nIMAGES\n\n") | ||
| if truncate { | ||
| fmt.Printf("%-12s %-64s\n", "IMAGE ID", "IMAGE NAME") |
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.
do you want to do the full 64 for the name or a shorter version 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.
Since we are trying to stay within 80 chars, I thought we would just do 64, also the Name is somewhat important while the ID is pretty much only used by the computer.
We have a bug in this patch in that you can not use the short name for delete operations.
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.
That's works for me, especially given the bug, I asked because I thought you shortened it at line 93. However i need to read more carefully, you didn't shorten it there either.
TomSweeneyRedHat
left a comment
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.
LGTM, just a nit and a question.
|
@nalind PTAL. This one is a little more controversial in that we are expanding the list command to handle both containers and images. |
|
@TomSweeneyRedHat Could you take a look at getting the delete command to handle the short names? |
|
Sure, I'll see what I can mess up with short name deletes. |
|
Will rename buildah list to |
This patch set will cleanup the output of buildah list adding truncate
flags as well as support images.