Deprecated -t for --template to make it --tty #12813

janetkuo · 2015-08-17T18:58:02Z

janetkuo · 2015-08-17T18:58:08Z

k8s-bot · 2015-08-17T19:41:11Z

GCE e2e build/test passed for commit 12f92a6c646bc5db1cd5d0aae32cd0885320022f.

bgrant0607 · 2015-08-17T21:56:51Z

docs/man/man1/kubectl-annotate.1

@@ -63,8 +63,8 @@ resourcequotas (quota) or secrets.

 .PP
 \fB\-t\fP, \fB\-\-template\fP=""
-    Template string or path to template file to use when \-o=template or \-o=templatefile.  The template format is golang templates [
-\[la]http://golang.org/pkg/text/template/#pkg-overview\[ra]]
+    Template string or path to template file to use when \-o=template or \-o=templatefile.  \-t is DEPRECATED to support \-\-tty.  The template format is golang templates [


I wouldn't mention that we plan to use -t for --tty -- that may just confuse users.

bgrant0607 · 2015-08-17T21:57:27Z

cc @kubernetes/kubectl

k8s-bot · 2015-08-18T00:07:07Z

GCE e2e build/test passed for commit 59738f2fa451505913c84153d2c0ada22b312373.

smarterclayton · 2015-08-18T00:54:11Z

No objections from me

deads2k · 2015-08-18T12:42:49Z

pkg/kubectl/cmd/util/printing.go

-	if len(outputFormat) == 0 && len(templateFile) != 0 {
-		outputFormat = "template"
+	if len(templateFile) != 0 {
+		glog.Warningf("%s is DEPRECATED and will be removed in a future version.", "-t")


Is there way for you to tell whether they've typed -t versus --template. If I'm reading the issue right, the intent is to deprecate -t, but not --template. I'm concerned about dumping this message too aggressively.

One alternative is to actually split --template into two flags and use MarkDeprecated for the -t flag. If you do that, pflags will take care of the deprecation notification.

janetkuo · 2015-08-18T23:53:39Z

@deads2k Thanks for your suggestions. I didn't find a way that tells whether the users typed the shorthand or the flag name. Therefore, I updated the code to read the args and see if a deprecated shorthand, such as -t, is used, and then prints deprecated warning message. This way, we only output deprecated warning when -t is used (but not --template). (The code is a bit hacky though.)

As for the alternative you mentioned, splitting the flag gives us --template and --t, but not -t.

k8s-bot · 2015-08-19T00:11:16Z

GCE e2e build/test failed for commit 677a437daa96aef5173e3685e3c3ade408923a57.

janetkuo · 2015-08-19T00:57:50Z

@k8s-bot test this please

k8s-bot · 2015-08-19T01:18:29Z

GCE e2e build/test passed for commit 677a437daa96aef5173e3685e3c3ade408923a57.

deads2k · 2015-08-19T11:45:22Z

pkg/kubectl/cmd/util/factory.go

@@ -303,6 +307,34 @@ func (f *Factory) BindFlags(flags *pflag.FlagSet) {
 	flags.SetNormalizeFunc(util.WordSepNormalizeFunc)
 }

+func (f *Factory) AddDeprecatedFlagShorthands(shorthand string) error {


How about moving this code into pflag? It looks like this would be a good complementary method to https://github.com/kubernetes/kubernetes/blob/master/Godeps/_workspace/src/github.com/spf13/pflag/flag.go#L283 and detection you wrote below would function well inside https://github.com/kubernetes/kubernetes/blob/master/Godeps/_workspace/src/github.com/spf13/pflag/flag.go#L299 against the pre-normalized name. The details (flag.Deprecated) could be used to explain that only -t is gone.

We have a couple committers on pflags. @eparis Any objections to that approach instead of adding this here?

I think that's a good idea to move this to pflag. Doing it "right" will take a bit more work. Obviously right now we only track if the short and long are both deprecated. So we'll need a way to say that only the short is deprecated. We really should start hiding the short from help text and documentation generation. I don't think Set() is actually the place you'll be able to find the short flag. I think it only handles long flags. You'll need to look at origArg[0] in setFlag().

Also CheckDeprecatedFlagShorthands() isn't right... It is legal (although silly) to do something like -wt '{ name }' (or something like there) where you have multiple shorthands after one -

Thanks @deads2k @eparis, I'll create a separate PR in github.com/spf13/pflag to support deprecating flag shorthands.

eparis · 2015-08-19T16:55:49Z

@deads2k I also thought about the split idea but we'd need an options struct like we talked about yesterday....

ghodss · 2015-08-19T18:30:17Z

Are we removing the golang template functionality altogether, or moving it to --golang-template or somehwere else? If we're moving it, it probably doesn't make sense to tell people it's being deprecated without giving them the future way of doing it.

janetkuo · 2015-08-19T20:17:45Z

@ghodss We're only deprecating -t but not --template.

ghodss · 2015-08-19T21:22:04Z

@janetkuo ah cool. I personally would recommend the help text be changed slightly to something like: -t is DEPRECATED and will be removed in a future version - please use --template. since to me the text reads more like the feature is deprecated.

janetkuo · 2015-08-19T23:05:28Z

Godeps/_workspace/src/github.com/spf13/pflag/flag.go

@@ -289,6 +290,16 @@ func (f *FlagSet) MarkDeprecated(name string, usageMessage string) error {
 	return nil
 }

+// Mark the shorthand of a flag deprecated in your program
+func (f *FlagSet) MarkShorthandDeprecated(name string, usageMessage string) error {


Code updated manually. It's easier to test the code change here. @deads2k @eparis please take a look and I'll apply this change to pflag package.

Code updated manually. It's easier to test the code change here. @deads2k @eparis please take a look and I'll apply this change to pflag package.

The pflags change looks good. Can you separate this into its own commit? I'd suggest naming it something like "UPSTREAM: pflags 52: Add shorthand deprecator". It makes it easier for people to find patches to bumped godeps.

k8s-bot · 2015-08-19T23:29:51Z

GCE e2e build/test passed for commit 4fc78b499339532752d32d84c3bb33a94129dde4.

k8s-bot · 2015-08-21T07:47:26Z

GCE e2e build/test passed for commit adc87307caaa44af654c009202c10fdce11e15a1.

deads2k · 2015-08-21T11:34:38Z

Looks like you just need to update the generated docs and squash the non-upstream changes.

lgtm

k8s-bot · 2015-08-21T17:00:51Z

GCE e2e build/test passed for commit 3e0903554b79713914fc42e8d16c5154ac8a8883.

k8s-bot · 2015-08-21T17:33:23Z

GCE e2e build/test passed for commit 9e92cd55f83f864dd4db5980b1e7363928f85f1c.

k8s-bot · 2015-08-21T19:07:21Z

GCE e2e build/test passed for commit 0436eccfa4f953dffb3acbc392cc3a66b402176e.

deads2k · 2015-08-21T20:01:55Z

lgtm. @jlowdermilk can you tag?

janetkuo · 2015-08-24T10:57:19Z

@k8s-bot test this please

k8s-bot · 2015-08-24T14:21:17Z

GCE e2e build/test passed for commit 0bcb314.

eparis · 2015-08-24T14:30:36Z

Just to be clear. The LGTM label stands even after the rebase. Ready to merge.

Deprecated -t for --template to make it --tty

Automatic merge from submit-queue Add flag -t as shorthand for --tty `-t` was deprecated in #12813 (Aug. 2015, about 6+ months ago). Now remove `--template`'s shorthand `-t` and create a shorthand `-t` for `--tty` in `kubectl run`. @kubernetes/kubectl

googlebot added the cla: yes label Aug 17, 2015

janetkuo added the area/kubectl label Aug 17, 2015

roberthbailey assigned bgrant0607 Aug 17, 2015

bgrant0607 reviewed Aug 17, 2015
View reviewed changes

bgrant0607 assigned deads2k and unassigned bgrant0607 Aug 17, 2015

janetkuo force-pushed the kubectl-deprecate-t branch from 12f92a6 to 59738f2 Compare August 17, 2015 23:44

deads2k reviewed Aug 18, 2015
View reviewed changes

janetkuo force-pushed the kubectl-deprecate-t branch from 59738f2 to 677a437 Compare August 18, 2015 23:45

deads2k reviewed Aug 19, 2015
View reviewed changes

janetkuo force-pushed the kubectl-deprecate-t branch from 677a437 to 4fc78b4 Compare August 19, 2015 23:00

janetkuo reviewed Aug 19, 2015
View reviewed changes

googlebot added cla: no and removed cla: yes labels Aug 21, 2015

janetkuo force-pushed the kubectl-deprecate-t branch from adc8730 to 3e09035 Compare August 21, 2015 16:39

googlebot added cla: yes and removed cla: no labels Aug 21, 2015

janetkuo force-pushed the kubectl-deprecate-t branch from 3e09035 to 9e92cd5 Compare August 21, 2015 17:09

janetkuo force-pushed the kubectl-deprecate-t branch from 9e92cd5 to 0436ecc Compare August 21, 2015 18:42

janetkuo added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 21, 2015

janetkuo added 2 commits August 24, 2015 02:15

UPSTREAM: pflags 52: Add shorthand deprecator

4047e13

Deprecated -t for --template to make it --tty

0bcb314

janetkuo force-pushed the kubectl-deprecate-t branch from 0436ecc to 0bcb314 Compare August 24, 2015 09:16

nikhiljindal added a commit that referenced this pull request Aug 24, 2015

Merge pull request #12813 from JanetKuo/kubectl-deprecate-t

4ffb59f

Deprecated -t for --template to make it --tty

nikhiljindal merged commit 4ffb59f into kubernetes:master Aug 24, 2015

daizuozhuo mentioned this pull request Aug 27, 2015

change -o template to -o go-template=... #13031

Merged

janetkuo added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Sep 14, 2015

zoidyzoidzoid mentioned this pull request Dec 2, 2015

k8s: -t for --template is deprecated since v1.1.1 vitessio/vitess#1360

Merged

janetkuo mentioned this pull request Apr 16, 2016

Add flag -t as shorthand for --tty #24365

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecated -t for --template to make it --tty #12813

Deprecated -t for --template to make it --tty #12813

janetkuo commented Aug 17, 2015

janetkuo commented Aug 17, 2015

k8s-bot commented Aug 17, 2015

bgrant0607 Aug 17, 2015

janetkuo Aug 17, 2015

bgrant0607 commented Aug 17, 2015

k8s-bot commented Aug 18, 2015

smarterclayton commented Aug 18, 2015

deads2k Aug 18, 2015

janetkuo commented Aug 18, 2015

k8s-bot commented Aug 19, 2015

janetkuo commented Aug 19, 2015

k8s-bot commented Aug 19, 2015

deads2k Aug 19, 2015

eparis Aug 19, 2015

janetkuo Aug 19, 2015

eparis commented Aug 19, 2015

ghodss commented Aug 19, 2015

janetkuo commented Aug 19, 2015

ghodss commented Aug 19, 2015

janetkuo Aug 19, 2015

deads2k Aug 20, 2015

k8s-bot commented Aug 19, 2015

k8s-bot commented Aug 21, 2015

deads2k commented Aug 21, 2015

k8s-bot commented Aug 21, 2015

k8s-bot commented Aug 21, 2015

k8s-bot commented Aug 21, 2015

deads2k commented Aug 21, 2015

janetkuo commented Aug 24, 2015

k8s-bot commented Aug 24, 2015

eparis commented Aug 24, 2015

Deprecated -t for --template to make it --tty #12813

Deprecated -t for --template to make it --tty #12813

Conversation

janetkuo commented Aug 17, 2015

janetkuo commented Aug 17, 2015

k8s-bot commented Aug 17, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgrant0607 commented Aug 17, 2015

k8s-bot commented Aug 18, 2015

smarterclayton commented Aug 18, 2015

Choose a reason for hiding this comment

janetkuo commented Aug 18, 2015

k8s-bot commented Aug 19, 2015

janetkuo commented Aug 19, 2015

k8s-bot commented Aug 19, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eparis commented Aug 19, 2015

ghodss commented Aug 19, 2015

janetkuo commented Aug 19, 2015

ghodss commented Aug 19, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-bot commented Aug 19, 2015

k8s-bot commented Aug 21, 2015

deads2k commented Aug 21, 2015

k8s-bot commented Aug 21, 2015

k8s-bot commented Aug 21, 2015

k8s-bot commented Aug 21, 2015

deads2k commented Aug 21, 2015

janetkuo commented Aug 24, 2015

k8s-bot commented Aug 24, 2015

eparis commented Aug 24, 2015