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

Projects
None yet
5 participants
@dnephin
Collaborator

dnephin commented Oct 26, 2017

Follow up to #642

Hide help flag from help output.
Signed-off-by: Daniel Nephin <dnephin@docker.com>

@dnephin dnephin requested a review from thaJeztah Oct 26, 2017

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io 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

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

This comment has been minimized.

@thaJeztah

thaJeztah Oct 26, 2017

Member

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

@thaJeztah

thaJeztah Oct 26, 2017

Member

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

This comment has been minimized.

@dnephin

dnephin Oct 26, 2017

Collaborator

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.

@dnephin

dnephin Oct 26, 2017

Collaborator

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.

@vdemeester

LGTM 🐮

@thaJeztah

LGTM, thanks for explaining!

@thaJeztah thaJeztah merged commit 6aedafd into docker:master Oct 27, 2017

7 of 8 checks passed

codecov/patch 0% of diff hit (target 50%)
Details
ci/circleci: cross Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: shellcheck Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
ci/circleci: validate Your tests passed on CircleCI!
Details
codecov/project 49.39% (-0.01%) compared to e59f3e2
Details
continuous-integration/jenkins/pr-head This commit looks good
Details

@thaJeztah thaJeztah added this to the 17.11.0 milestone Oct 27, 2017

@dnephin dnephin deleted the dnephin:use-upstream-spf13-cobra branch Oct 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment