Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide help flag from help output #645

Merged
merged 1 commit into from
Oct 27, 2017

Conversation

dnephin
Copy link
Contributor

@dnephin dnephin commented Oct 26, 2017

Follow up to #642

Signed-off-by: Daniel Nephin <dnephin@docker.com>
@dnephin dnephin requested a review from thaJeztah October 26, 2017 16:21
@codecov-io
Copy link

codecov-io commented Oct 26, 2017

Codecov Report

Merging #645 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #645      +/-   ##
==========================================
- Coverage    49.4%   49.39%   -0.01%     
==========================================
  Files         208      208              
  Lines       17194    17195       +1     
==========================================
  Hits         8494     8494              
- Misses       8267     8268       +1     
  Partials      433      433

@@ -26,6 +26,7 @@ func SetupRootCommand(rootCmd *cobra.Command) {

rootCmd.PersistentFlags().BoolP("help", "h", false, "Print usage")
rootCmd.PersistentFlags().MarkShorthandDeprecated("help", "please use --help")
rootCmd.PersistentFlags().Lookup("help").Hidden = true
Copy link
Member

Choose a reason for hiding this comment

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

hm, didn't the .golden file need to be updated? https://github.com/docker/cli/pull/642/files#r147009761

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 expected it would, but it doesn't because of how the flags are applied.

This help flag is applied as a persistent flag to the root command, so when commands are added to the root, they inherit this version of the flag, and don't add the default.

In the test we are testing directly against the swarm update command, so the root command is not there to provide this flag. Cobra adds its own default help flag that is not hidden, so the test output still contains the --help flag.

Copy link
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐮

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for explaining!

@thaJeztah thaJeztah merged commit 6aedafd into docker:master Oct 27, 2017
@thaJeztah thaJeztah added this to the 17.11.0 milestone Oct 27, 2017
@dnephin dnephin deleted the use-upstream-spf13-cobra branch October 27, 2017 15:17
Sign up for free to 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.

5 participants