Skip to content
This repository has been archived by the owner on Jun 13, 2021. It is now read-only.

New command to list application images #639

Merged
merged 1 commit into from Sep 27, 2019

Conversation

rumpl
Copy link
Member

@rumpl rumpl commented Sep 25, 2019

For now we show the reference, name and version.

- What I did

Added a new management command image and a subcommand ls (or list)

- How I did it

Some of the code is from this branch made by Silvin, added some more unit and e2e tests.

- How to verify it

  • Bundle an app: docker app bundle myapp.dockerapp --tag myapp
  • Run docker app image ls

And you should see the application

- Description for the changelog
New subcommand docker app image ls to list docker applications that are in the local bundle store.

- A picture of a cute animal (not mandatory but encouraged)
image

@codecov
Copy link

codecov bot commented Sep 25, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@695509b). Click here to learn what that means.
The diff coverage is 78.44%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #639   +/-   ##
=========================================
  Coverage          ?   72.12%           
=========================================
  Files             ?       51           
  Lines             ?     2665           
  Branches          ?        0           
=========================================
  Hits              ?     1922           
  Misses            ?      502           
  Partials          ?      241
Impacted Files Coverage Δ
internal/commands/root.go 69.3% <100%> (ø)
internal/commands/image/command.go 100% <100%> (ø)
internal/store/bundle.go 68.81% <68.75%> (ø)
internal/commands/image/list.go 83.6% <83.6%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 695509b...aa2de75. Read the comment docs.

e2e/images_test.go Outdated Show resolved Hide resolved
@jcsirot
Copy link
Contributor

jcsirot commented Sep 26, 2019

LGTM but I think we should wait for #635 to be merged and then rebase this PR.

@rumpl rumpl force-pushed the feat-image-ls branch 3 times, most recently from 46a2606 to e3085b2 Compare September 26, 2019 15:57
)

func TestImageList(t *testing.T) {
runWithDindSwarmAndRegistry(t, func(info dindSwarmAndRegistryInfo) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As runWithDindSwarmAndRegistry is defined in e2e/pushpull_test.go maybe we can move the method in e2e/helper_test.go. This way this test only depends on the helper and not on an other test file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


func TestImageList(t *testing.T) {
runWithDindSwarmAndRegistry(t, func(info dindSwarmAndRegistryInfo) {
cmd, cleanup := dockerCli.createTestCmd()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not to use cmd := info.configuredCmd? And this way the cleanup is already called by runWithDindSwarmAndRegistry

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I blindly copied the code from another test... Thanks!

}
return ""
}},
{"APP NAME", func(p pkg) string { return p.bundle.Name }},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit (just to have everything written the same way):

Suggested change
{"APP NAME", func(p pkg) string { return p.bundle.Name }},
{"APP NAME", func(p pkg) string {
return p.bundle.Name
}},

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

For now we show the reference, name and version.

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@silvin-lubecki silvin-lubecki merged commit acbaab0 into docker:master Sep 27, 2019
@rumpl rumpl deleted the feat-image-ls branch October 18, 2019 08:35
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.

None yet

6 participants