Skip to content

Conversation

@ijc
Copy link
Contributor

@ijc ijc commented Apr 26, 2019

- What I did

I arranged to cli plugins include in the output of docker --badopt, fixes #1813.

In the process of writing the e2e test I also noticed that the docker --badopt unconditionally included experimental (builtin) commands so I fixed that too.

Finally since I was adding a new subtest to an existing test, I took the chance to use t.Run for all of the subtests.

- How I did it

Including cli plugins is achieved by calling pluginmanager.AddPluginCommandStubs in the flag errro callback function. Handling experimental commands properly is handled by calling hideUnsupportedFeatures in the same callback.

- How to verify it

There is a new e2e test.

- Description for the changelog

  • Help output now always includes CLI plugins.

/cc @SvenDowideit.

Ian Campbell added 3 commits April 26, 2019 10:43
Previously `docker --badopt` would always include experimental commands even if
experimental was not enabled.

Signed-off-by: Ian Campbell <[email protected]>
Previously `docker --badopt` output would not include CLI plugins.

Fixes docker#1813

Signed-off-by: Ian Campbell <[email protected]>
The linter is complaining:

    cmd/docker/docker.go:72:23:warning: dockerCli can be github.com/docker/cli/cli/command.Cli (interfacer)

Unclear precisely which change in the preceeding commits caused it to notice
this possibility.

Signed-off-by: Ian Campbell <[email protected]>
@codecov-io
Copy link

codecov-io commented Apr 26, 2019

Codecov Report

Merging #1850 into master will decrease coverage by 0.01%.
The diff coverage is 20%.

@@            Coverage Diff             @@
##           master    #1850      +/-   ##
==========================================
- Coverage   56.75%   56.74%   -0.02%     
==========================================
  Files         309      309              
  Lines       21658    21662       +4     
==========================================
  Hits        12292    12292              
- Misses       8469     8473       +4     
  Partials      897      897

@ijc
Copy link
Contributor Author

ijc commented Apr 26, 2019

Linter complained:

cmd/docker/docker.go:72:23:warning: dockerCli can be github.com/docker/cli/cli/command.Cli (interfacer)

Since I'm only adding new function calls to setFlagErrorFunc I'm not sure why it only realises this now, but I've added a commit to make that change.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@thaJeztah
Copy link
Member

ping @silvin-lubecki @tiborvass ptal

@SvenDowideit
Copy link
Contributor

noice! 👍 :)

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 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

calling docker --badopt doesn't show cli plugins

6 participants