Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

Add short description to svcat install and hide command in plugin mode #2050

Merged
merged 4 commits into from
May 18, 2018

Conversation

luksa
Copy link
Contributor

@luksa luksa commented May 18, 2018

No description provided.

luksa added 3 commits May 18, 2018 15:15
The install command only makes sense when invoked through svcat directly,
so it should not be included in "kubectl plugin svcat --help"
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 18, 2018
@luksa luksa changed the title Add short description to svcat install and hide it in plugin mode Add short description to svcat install and hide command in plugin mode May 18, 2018
@@ -171,7 +173,8 @@ func newDescribeCmd(cxt *command.Context) *cobra.Command {

func newInstallCmd(cxt *command.Context) *cobra.Command {
cmd := &cobra.Command{
Use: "install",
Use: "install",
Copy link
Contributor

Choose a reason for hiding this comment

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

The install subcommand is a placeholder for additional commands, e.g. installing service catalog to a cluster. So either we need some super generic text to explain it, or leave it as-is, without a short description so that people rely on the deeper commands (e.g. svcat install plugin --help) for doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't think of a better description for now. But leaving it empty is definitely not ok. If empty, it looks odd - as if we forgot to add a description:

$ svcat --help
The Kubernetes Service Catalog Command-Line Interface (CLI)

Usage:
  svcat [flags]
  svcat [command]

Available Commands:
  bind        Binds an instance's metadata to a secret, which can then be used by an application to connect to the instance
  completion  Output shell completion code for the specified shell (bash).
  deprovision Deletes an instance of a service
  describe    Show details of a specific resource
  get         List a resource, optionally filtered by name
  install     
  provision   Create a new instance of a service
  sync        Syncs service catalog for a service broker
  touch       Force Service Catalog to reprocess a resource
  unbind      Unbinds an instance. When an instance name is specified, all of its bindings are removed, otherwise use --name to remove a specific binding
  version     Provides the version for the Service Catalog client and server

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe something like "Various commands for installing service catalog & related tools" ?

Copy link
Contributor

Choose a reason for hiding this comment

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

How about "Install service catalog related tools"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I've changed it. When we add the option of installing the service catalog itself, we can reword it.

BTW, I believe it should be "Install Service-Catalog-related tools", right?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not an editor but if you want to hyphenate it would be "Service Catalog-related" tools which looks odd to me. "Service Catalog" itself isn't hyphenated.

@n3wscott n3wscott added the LGTM1 label May 18, 2018
Copy link
Contributor

@carolynvs carolynvs left a comment

Choose a reason for hiding this comment

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

LGTM

@carolynvs carolynvs merged commit 9b8177a into kubernetes-retired:master May 18, 2018
@luksa luksa deleted the svcat_plugin_install branch May 19, 2018 10:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. LGTM1 LGTM2 size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants