Skip to content

Introduce plugins/list subcommand#2431

Merged
mlaventure merged 1 commit intocontainerd:masterfrom
masters-of-cats:plugins-list-subcommand
Jul 2, 2018
Merged

Introduce plugins/list subcommand#2431
mlaventure merged 1 commit intocontainerd:masterfrom
masters-of-cats:plugins-list-subcommand

Conversation

@danail-branekov
Copy link
Copy Markdown
Contributor

Ctr interface follows the pattern ctr <command> <subcommand> except
for the plugins command which does not have subcommands. This feels
unnatural to certain users and they would expect that they can list
containerd plugins via ctr plugins list.

This commit implements their expectation so that plugins becomes a
command "group" and its list subcommand actually lists the plugins.

Signed-off-by: Danail Branekov [email protected]

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jun 29, 2018

Codecov Report

Merging #2431 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2431   +/-   ##
======================================
  Coverage      45%     45%           
======================================
  Files          92      92           
  Lines        9412    9412           
======================================
  Hits         4236    4236           
  Misses       4493    4493           
  Partials      683     683
Flag Coverage Δ
#linux 49.23% <ø> (ø) ⬆️
#windows 41.3% <ø> (ø) ⬆️

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 a044b04...3cf3881. Read the comment docs.

@crosbymichael
Copy link
Copy Markdown
Member

LGTM

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread cmd/ctr/commands/plugins/plugins.go Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Alias with ls like other list commands?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thought of that just as I clicked Approve :-} I think that's a good suggestion--can we get that small change and then we can merge?

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.

Sure, commit updated

`Ctr` interface follows the pattern `ctr <command> <subcommand>` except
for the `plugins` command which does not have subcommands. This feels
unnatural to certain users and they would expect that they can list
containerd plugins via `ctr plugins list`.

This commit implements their expectation so that `plugins` becomes a
command "group" and its `list` subcommand actually lists the plugins.

Signed-off-by: Danail Branekov <[email protected]>
@danail-branekov danail-branekov force-pushed the plugins-list-subcommand branch from edfd24a to 3cf3881 Compare July 2, 2018 07:21
Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@mlaventure mlaventure left a comment

Choose a reason for hiding this comment

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

LGTM

@mlaventure mlaventure merged commit 39b6ba8 into containerd:master Jul 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants