-
Notifications
You must be signed in to change notification settings - Fork 531
Change shorthand --enable-type flag to avoid typo #1121
Change shorthand --enable-type flag to avoid typo #1121
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: font The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@font
The shorthand of other related options are the first charactor of the long name for easy memorized.
What about change the long name to make it consistent with others?
e.g. s/enable-type/type-enabled/
@xunpan Yes, that's the downside i.e. the shorthand letter is not the first character of the long name. Thanks for the suggestion. I think it's preferable to stick with |
Yeah, agree with @font - the verb-noun form is more intuitive and consistent IMO. But good suggestion nevertheless @xunpan. |
/lgtm |
@abhat: changing LGTM is restricted to collaborators In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
It is not a mandatory change but suggestion. I'm ok with your comments. LGTM |
/lgtm |
What this PR does / why we need it:
The
--enable-type
long option can be accidentally typed with only 1-
as in-enable-type
resulting in the following error:The error occurs because combined shorthand options as in
-en
are valid. If a shorthand option takes an argument, as in-n
, it must be the last argument, so it interprets the rest of the commandable-type
as the namespace argument. This can be confusing and is only a special case becausee
andn
are both shorthand flags while their combined use spells out the long form of an option.This fixes that by changing the shorthand flag for
--enable-type
to-t
instead of-e
.Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #
Special notes for your reviewer: