fix some cli command description #12128

guangxuli · 2016-12-03T03:10:16Z

what this PR does:

Mainly fix some description of some oc command, especially for the default value of option is false.

when i run oc deploy -h, the output :

Options:
      --cancel=false: Cancel the in-progress deployment.
      --enable-triggers=false: Enables all image triggers for the deployment config.
      --follow=false: Follow the logs of a deployment
      --latest=false: Start a new deployment now.
      --retry=false: Retry the latest failed deployment.

I think the annotation of the options is confusing, since all the description are about * = true condition,
while the default value of these options are false. I thought this is our rules at beginning, but when I run oc new-build -h command, I saw some outputs are more reasonable, like this(not all):

Options:
      --allow-missing-images=false: If true, indicates that referenced Docker images that cannot be found locally or in
a registry should still be used.
      --allow-missing-imagestream-tags=false: If true, indicates that image stream tags that don't exist should still be
used.
      --dry-run=false: If true, show the result of the operation without performing it.
      --no-output=false: If true, the build output will not be pushed anywhere.

I think we should add if true when the bool value of description before colons is false, that seems more clear.

what I have fixed:

option=false, but the description is about in case of the value is true, add if true to the description if not exist:

cmd.Flags().BoolVar(&o.Confirmed, "confirm", o.Confirmed, "**Specify** that cluster role bindings should be modified. Defaults to false, displaying what would be replaced but not actually replacing anything.")
cmd.Flags().BoolVar(&o.Confirmed, "confirm", o.Confirmed, "**If true, specify** that cluster role bindings should be modified. Defaults to false, displaying what would be replaced but not actually replacing anything.")

description if present change it to if true just for keeping consistent:

cmd.Flags().Bool("all-namespaces", false, "**If present,** list the requested object(s) across all namespaces. Namespace in current context is ignored even if specified with --namespace.")
cmd.Flags().Bool("all-namespaces", false, "**If true**, list the requested object(s) across all namespaces. Namespace in current context is ignored even if specified with --namespace.")

what I do not fixed:

description include all false/true, just like:

cmd.Flags().BoolVar(&options.Readiness, "readiness", options.Readiness, "Set or remove a readiness probe to indicate when this container should receive traffic")
cmd.Flags().BoolVar(&options.Liveness, "liveness", options.Liveness, "Set or remove a liveness probe to verify this container is running")

description have include clear option content, just like:

cmd.Flags().BoolVar(&opts.aliasTag, "alias", false, "Should the destination tag be updated whenever the source tag changes. **Defaults to false**.")cmd.Flags().BoolVar(&opts.referenceTag, "reference", false, "Should the destination tag continue to pull from the source namespace. **Defaults to false**.")

3.the option have two format, just like:
-t, --tty=false: Force a pseudo-terminal to be allocated

pweil- · 2016-12-06T13:50:49Z

@openshift/cli-review

fabianofranz · 2016-12-06T15:18:13Z

[test]

guangxuli · 2016-12-07T14:54:02Z

@fabianofranz ptal,thanks.

guangxuli · 2016-12-08T03:36:56Z

@fabianofranz I check the results of test failure, it seems a flake. Can you trigger test again? thanks.

juanvallejo · 2016-12-08T14:48:56Z

check flaked on #12157 re[test]

guangxuli · 2016-12-08T15:19:42Z

@juanvallejo thanks.:)

guangxuli · 2016-12-09T04:27:50Z

@juanvallejo test failure again, seems another flake.

juanvallejo · 2016-12-09T16:40:47Z

integration check flaked on #8571
re[test]

@guangxuli looks like you'll need to rebase as well

guangxuli · 2016-12-12T01:19:31Z

@juanvallejo thanks so much, have rebased.

juanvallejo · 2016-12-12T16:09:05Z

@fabianofranz lgtm

re[test]

fabianofranz · 2016-12-12T18:10:37Z

flaked on #8502 re[test]

fabianofranz · 2016-12-12T20:01:17Z

flaked on #8502 re[test]

guangxuli · 2016-12-13T02:09:20Z

@fabianofranz @juanvallejo seems a flake again

juanvallejo · 2016-12-13T14:41:17Z

integration check flaked on #11775 re[test]

fabianofranz · 2016-12-13T18:56:05Z

pkg/cmd/cli/cmd/set/volume.go

@@ -171,15 +171,15 @@ func NewCmdVolume(fullName string, f *clientcmd.Factory, out, errOut io.Writer)
 		},
 	}
 	cmd.Flags().StringVarP(&opts.Selector, "selector", "l", "", "Selector (label query) to filter on")
-	cmd.Flags().BoolVar(&opts.All, "all", false, "select all resources in the namespace of the specified resource types")
+	cmd.Flags().BoolVar(&opts.All, "all", false, "If true, Select all resources in the namespace of the specified resource types")


*select must be lowercase.

will be fixed, thanks.

fabianofranz · 2016-12-13T18:59:06Z

pkg/cmd/admin/router/router.go

@@ -270,7 +270,7 @@ func NewCmdRouter(f *clientcmd.Factory, parentName, name string, out, errout io.
 	cmd.Flags().IntVar(&cfg.StatsPort, "stats-port", cfg.StatsPort, "If the underlying router implementation can provide statistics this is a hint to expose it on this port. Specify 0 if you want to turn off exposing the statistics.")
 	cmd.Flags().StringVar(&cfg.StatsPassword, "stats-password", cfg.StatsPassword, "If the underlying router implementation can provide statistics this is the requested password for auth.  If not set a password will be generated.")
 	cmd.Flags().StringVar(&cfg.StatsUsername, "stats-user", cfg.StatsUsername, "If the underlying router implementation can provide statistics this is the requested username for auth.")
-	cmd.Flags().BoolVar(&cfg.ExposeMetrics, "expose-metrics", cfg.ExposeMetrics, "This is a hint to run an extra container in the pod to expose metrics - the image will either be set depending on the router implementation or provided with --metrics-image.")
+	cmd.Flags().BoolVar(&cfg.ExposeMetrics, "expose-metrics", cfg.ExposeMetrics, "If true, this is a hint to run an extra container in the pod to expose metrics - the image will either be set depending on the router implementation or provided with --metrics-image.")


"f true, attempts to run ..."

agreed. thanks.

fabianofranz · 2016-12-13T18:59:32Z

A couple minor nits then LGTM.

guangxuli · 2016-12-14T00:47:53Z

@fabianofranz done. PTAL, thanks.

fabianofranz · 2016-12-14T14:49:54Z

Needs a rebase.

guangxuli · 2016-12-14T15:36:50Z

@fabianofranz have rebased.

guangxuli · 2016-12-14T17:16:46Z

@fabianofranz PTAL,thanks.

fabianofranz · 2016-12-14T17:35:37Z

Thanks @guangxuli! LGTM [merge]

openshift-bot · 2016-12-14T17:37:19Z

Evaluated for origin merge up to 9c93da2

openshift-bot · 2016-12-14T17:37:20Z

[Test]ing while waiting on the merge queue

openshift-bot · 2016-12-14T17:45:21Z

Evaluated for origin test up to 9c93da2

openshift-bot · 2016-12-14T19:05:31Z

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/12384/) (Base Commit: 652b30b)

openshift-bot · 2016-12-15T01:05:30Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/12400/) (Base Commit: 039e433) (Image: devenv-rhel7_5548)

guangxuli force-pushed the openshift_whoami_print branch from eecdea5 to 05924ac Compare December 6, 2016 07:32

pweil- added priority/P2 component/cli kind/enhancement labels Dec 6, 2016

pweil- assigned fabianofranz Dec 6, 2016

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 9, 2016

guangxuli force-pushed the openshift_whoami_print branch from 05924ac to e3348a8 Compare December 12, 2016 01:14

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 12, 2016

fabianofranz reviewed Dec 13, 2016

View reviewed changes

guangxuli force-pushed the openshift_whoami_print branch from e3348a8 to b162ac1 Compare December 14, 2016 00:45

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 14, 2016

guangxuli force-pushed the openshift_whoami_print branch 2 times, most recently from f5fad1c to 34069ee Compare December 14, 2016 06:53

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 14, 2016

guangxuli force-pushed the openshift_whoami_print branch 2 times, most recently from 6ca4304 to 8116c1e Compare December 14, 2016 08:13

rebase

9c93da2

guangxuli force-pushed the openshift_whoami_print branch from 8116c1e to 9c93da2 Compare December 14, 2016 15:26

openshift-bot merged commit 46585f4 into openshift:master Dec 15, 2016

guangxuli deleted the openshift_whoami_print branch March 11, 2017 06:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix some cli command description #12128

fix some cli command description #12128

guangxuli commented Dec 3, 2016 •

edited

Loading

pweil- commented Dec 6, 2016

fabianofranz commented Dec 6, 2016

guangxuli commented Dec 7, 2016

guangxuli commented Dec 8, 2016

juanvallejo commented Dec 8, 2016

guangxuli commented Dec 8, 2016

guangxuli commented Dec 9, 2016

juanvallejo commented Dec 9, 2016

guangxuli commented Dec 12, 2016

juanvallejo commented Dec 12, 2016

fabianofranz commented Dec 12, 2016

fabianofranz commented Dec 12, 2016

guangxuli commented Dec 13, 2016

juanvallejo commented Dec 13, 2016

fabianofranz Dec 13, 2016

guangxuli Dec 14, 2016

fabianofranz Dec 13, 2016

guangxuli Dec 14, 2016

fabianofranz commented Dec 13, 2016

guangxuli commented Dec 14, 2016

fabianofranz commented Dec 14, 2016

guangxuli commented Dec 14, 2016

guangxuli commented Dec 14, 2016

fabianofranz commented Dec 14, 2016

openshift-bot commented Dec 14, 2016

openshift-bot commented Dec 14, 2016

openshift-bot commented Dec 14, 2016

openshift-bot commented Dec 14, 2016

openshift-bot commented Dec 15, 2016 •

edited

Loading

fix some cli command description #12128

fix some cli command description #12128

Conversation

guangxuli commented Dec 3, 2016 • edited Loading

pweil- commented Dec 6, 2016

fabianofranz commented Dec 6, 2016

guangxuli commented Dec 7, 2016

guangxuli commented Dec 8, 2016

juanvallejo commented Dec 8, 2016

guangxuli commented Dec 8, 2016

guangxuli commented Dec 9, 2016

juanvallejo commented Dec 9, 2016

guangxuli commented Dec 12, 2016

juanvallejo commented Dec 12, 2016

fabianofranz commented Dec 12, 2016

fabianofranz commented Dec 12, 2016

guangxuli commented Dec 13, 2016

juanvallejo commented Dec 13, 2016

fabianofranz Dec 13, 2016

Choose a reason for hiding this comment

guangxuli Dec 14, 2016

Choose a reason for hiding this comment

fabianofranz Dec 13, 2016

Choose a reason for hiding this comment

guangxuli Dec 14, 2016

Choose a reason for hiding this comment

fabianofranz commented Dec 13, 2016

guangxuli commented Dec 14, 2016

fabianofranz commented Dec 14, 2016

guangxuli commented Dec 14, 2016

guangxuli commented Dec 14, 2016

fabianofranz commented Dec 14, 2016

openshift-bot commented Dec 14, 2016

openshift-bot commented Dec 14, 2016

openshift-bot commented Dec 14, 2016

openshift-bot commented Dec 14, 2016

openshift-bot commented Dec 15, 2016 • edited Loading

guangxuli commented Dec 3, 2016 •

edited

Loading

openshift-bot commented Dec 15, 2016 •

edited

Loading