Skip to content

Add support for images as well as containers to buildah list & delete#47

Closed
rhatdan wants to merge 1 commit into
podman-container-tools:masterfrom
rhatdan:images
Closed

Add support for images as well as containers to buildah list & delete#47
rhatdan wants to merge 1 commit into
podman-container-tools:masterfrom
rhatdan:images

Conversation

@rhatdan

@rhatdan rhatdan commented Mar 27, 2017

Copy link
Copy Markdown
Contributor

This patch set will cleanup the output of buildah list adding truncate
flags as well as support images.

@rhatdan

rhatdan commented Mar 27, 2017

Copy link
Copy Markdown
Contributor Author

Alternative to #32

Comment thread 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)

Copy link
Copy Markdown
Contributor

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.
Comment thread cmd/buildah/list.go
if len(images) > 0 && !quiet {
fmt.Printf("\nIMAGES\n\n")
if truncate {
fmt.Printf("%-12s %-64s\n", "IMAGE ID", "IMAGE NAME")

Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Contributor

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 TomSweeneyRedHat left a comment

Copy link
Copy Markdown
Contributor

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.

@rhatdan

rhatdan commented Mar 27, 2017

Copy link
Copy Markdown
Contributor Author

@nalind PTAL. This one is a little more controversial in that we are expanding the list command to handle both containers and images.

@rhatdan

rhatdan commented Mar 27, 2017

Copy link
Copy Markdown
Contributor Author

@TomSweeneyRedHat Could you take a look at getting the delete command to handle the short names?

@TomSweeneyRedHat

Copy link
Copy Markdown
Contributor

Sure, I'll see what I can mess up with short name deletes.

@rhatdan rhatdan closed this Mar 28, 2017
@rhatdan

rhatdan commented Mar 28, 2017

Copy link
Copy Markdown
Contributor Author

Will rename buildah list to
buildah containers
and
buildah images.

nalind pushed a commit that referenced this pull request Nov 28, 2017
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Nov 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants