Skip to content

Reformat plugin's vendor position and add version on --help - #1675

Merged
silvin-lubecki merged 1 commit into
docker:masterfrom
ulyssessouza:format-plugin-vendor-version-help
Feb 25, 2019
Merged

Reformat plugin's vendor position and add version on --help#1675
silvin-lubecki merged 1 commit into
docker:masterfrom
ulyssessouza:format-plugin-vendor-version-help

Conversation

@ulyssessouza

@ulyssessouzaulyssessouza commented Feb 15, 2019

Copy link
Copy Markdown
Contributor

Checks the fourth box on: #1661

- What I did

  • Refactor plugins' vendor location on --help by relocating it at the end of the short description
  • Add ", " to command vendor

- How I did it
By changing the usageTemplate in cli/cobra.go to replace the information and decorating the command with a '*' when it's a plugin's first level command

- How to verify it

  • Install a plugin (or make your configuration point to one)
  • Execute docker help and check that the plugins' first level commands have now a '*' and it's vendor as short description suffix

- Description for the changelog
Reformated plugins' first level command description on --help by replacing and vendor and adding version

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

@codecov-io

codecov-io commented Feb 15, 2019

Copy link
Copy Markdown

Codecov Report

Merging #1675 into master will increase coverage by 0.07%.
The diff coverage is 63.15%.

@@ Coverage Diff @@## master #1675 +/- ##
==========================================
+ Coverage 56.05% 56.13% +0.07% 
==========================================
Files 306 306 Lines 20981 20970 -11 ==========================================
+ Hits 11761 11771 +10 + Misses 8371 8346 -25 - Partials 849 853 +4

@thaJeztah

Copy link
Copy Markdown
Member

Giving this a quick spin;

DOCKER_HIDE_LEGACY_COMMANDS=1 docker --config=.docker-plugin-test --help
Usage:	docker [OPTIONS] COMMAND
A self-sufficient runtime for containers
Options:
--config string Location of client config files (default "/root/.docker")
-c, --context string Name of the context to use to connect to the daemon (overrides DOCKER_HOST env var and default context set with "docker context use")
-D, --debug Enable debug mode
-H, --host list Daemon socket(s) to connect to
-l, --log-level string Set the logging level ("debug"|"info"|"warn"|"error"|"fatal") (default "info")
--tls Use TLS; implied by --tlsverify
--tlscacert string Trust certs signed only by this CA (default "/root/.docker/ca.pem")
--tlscert string Path to TLS certificate file (default "/root/.docker/cert.pem")
--tlskey string Path to TLS key file (default "/root/.docker/key.pem")
--tlsverify Use TLS and verify the remote
-v, --version Print version information and quit
Management Commands:
builder Manage builds
checkpoint Manage checkpoints
config Manage Docker configs
container Manage containers
context Manage contexts
engine Manage the docker engine
image Manage images
network Manage networks
node Manage Swarm nodes
plugin Manage plugins
secret Manage Docker secrets
service Manage services
stack Manage Docker stacks
swarm Manage Swarm
system Manage Docker
trust Manage trust on Docker images
volume Manage volumes
Commands:
build Build an image from a Dockerfile
helloworld* A basic Hello World plugin for tests (Docker Inc., testing)
login Log in to a Docker registry
logout Log out from a Docker registry
run Run a commandin a new container
search Search the Docker Hub for images
version Show the Docker version information
Run 'docker COMMAND --help'for more information on a command.

The new presentation of the plugin looks good to me 🤗

@thaJeztah

Copy link
Copy Markdown
Member

I just realise that I may not have mentioned that on the other PR (and probably something to address separately);

Plugins (most likely) will have subcommands; if so, we should show plugins under Management Command (Alternatively Plugin Commands)

@ijcijc 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.

I had one small question, but overall looks good, thanks for picking this up!

Comment threadcli/cobra.go Outdated
@ijc

ijc commented Feb 18, 2019

Copy link
Copy Markdown
Contributor

Plugins (most likely) will have subcommands; if so, we should show plugins under Management Command (Alternatively Plugin Commands)

Ack, I think that's separate from this change though, #1661 has a separate entry for it:

plugins shuld be able to indicate if they are "management" vs not commands in their metadata and be listed accordingly in e.g. docker help.

@thaJeztah

Copy link
Copy Markdown
Member

Ack, I think that's separate from this change though, #1661 has a separate entry for it:

Ah, yes. I started looking why it wouldn't see .HasSubCommands() but the plugin "proxy" commands don't know if a plugin command has subcommands (yet)

Comment threadcli/cobra.go Outdated
@ijcijc mentioned this pull request Feb 18, 2019
11 tasks
@ijc

ijc commented Feb 20, 2019

Copy link
Copy Markdown
Contributor

@ulyssessouza I think a Unit terst for decoratedCommandName would increase the score here. Don't know if it would hit the 50% target but nothing else looks to be sanely unit testable to me.

@ulyssessouza
ulyssessouzaforce-pushed the format-plugin-vendor-version-help branch 3 times, most recently from d6a05e5 to a6b7d5fCompareFebruary 21, 2019 15:41
Comment threadcli/cobra.go Outdated
Comment threadcli/cobra.go Outdated
Comment threadcli/cobra.go Outdated
Comment threadcli/cobra.go Outdated
Comment threadcli/cobra.go Outdated
Comment threadcli/cobra.go Outdated
Comment threadcli/cobra.go Outdated
Comment threadcli/cobra_test.go Outdated
@ulyssessouza
ulyssessouzaforce-pushed the format-plugin-vendor-version-help branch 3 times, most recently from 1d7e053 to 35c7809CompareFebruary 21, 2019 16:46

@thaJeztahthaJeztah left a comment

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.

LGTM

left one nit

Comment threadcli/cobra_test.go Outdated
- The placement of the vendor is now in the end of the line.
- A '*' is now added as suffix of plugins' top level commands.
Signed-off-by: Ulysses Souza <ulysses.souza@docker.com>
@ulyssessouza
ulyssessouzaforce-pushed the format-plugin-vendor-version-help branch from 35c7809 to 9201360CompareFebruary 21, 2019 16:54
ijc
ijc approved these changes Feb 22, 2019

@silvin-lubeckisilvin-lubecki 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

@silvin-lubecki
silvin-lubecki merged commit 11985c6 into docker:masterFeb 25, 2019
@GordonTheTurtleGordonTheTurtle added this to the 19.03.0 milestone Feb 25, 2019
@ulyssessouza
ulyssessouza deleted the format-plugin-vendor-version-help branch February 28, 2019 14:44
Sign up for freeto 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.

6 participants

@ulyssessouza@codecov-io@thaJeztah@ijc@silvin-lubecki@GordonTheTurtle