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.

Sign up for GitHub

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

Jump to bottom

Update github.com/spf13/pflag so that --version keeps working. #3740

Merged
merged 1 commit into from
Jan 22, 2015
Merged

Update github.com/spf13/pflag so that --version keeps working. #3740

merged 1 commit into from
Jan 22, 2015
+183 −3

Conversation

filbranden
Copy link
Contributor

Tested to fix the previous behavior of --version and --version=raw.

@jbeda

@filbranden
Update github.com/spf13/pflag to include spf13/pflag#3
374f997 
We need support for IsBoolFlag() in order to implement `--version`
(without an explicit `--version=true`) and also `--version=raw`.

Updated using `godep update github.com/spf13/pflag`.

Tested:
  $ _output/local/bin/linux/amd64/kubelet --version
  Kubernetes v0.8.0-590-gc61720437ec181
  $ _output/local/bin/linux/amd64/kubelet --version=raw
  version.Info{Major:"0", Minor:"8+", GitVersion:"v0.8.0-590-gc61720437ec181", GitCommit:"c61720437ec181ae3fbbfb9d6df5efdf3e3b6c2e", GitTreeState:"clean"}
@filbranden filbranden mentioned this pull request Jan 22, 2015
Closed
jbeda added a commit that referenced this pull request Jan 22, 2015
@jbeda
Merge pull request #3740 from filbranden/verflag_new_spf13_pflag1
212ba0f 
Update github.com/spf13/pflag so that --version keeps working.
@jbeda jbeda merged commit 212ba0f into kubernetes:master Jan 22, 2015
@filbranden filbranden deleted the verflag_new_spf13_pflag1 branch January 22, 2015 22:30
@thockin
Copy link
Member

thockin commented Jan 22, 2015

Would this manifest as "flag needs an argument: --logtostderr" ? Is this
supposed to work in the brave new world?

On Thu, Jan 22, 2015 at 2:27 PM, Filipe Brandenburger <
notifications@github.com> wrote:

Tested to fix the previous behavior of --version and --version=raw.

@jbeda https://github.com/jbeda

You can view, comment on, or merge this pull request online at:

#3740
Commit Summary

File Changes

Patch Links:

Reply to this email directly or view it on GitHub
#3740.

@filbranden
Copy link
Contributor Author

@thockin I suspect that might be another side effect of the flag -> pflag transition from #3480...

I'll take a look, hopefully it's something easy to fix. For now I guess --logtostderr=true should work.

Cheers,
Filipe

@thockin
Copy link
Member

thockin commented Jan 23, 2015

Thanks, I just want to be on record as saying that boolean flags need to
not take an argument :)

On Thu, Jan 22, 2015 at 3:57 PM, Filipe Brandenburger <
notifications@github.com> wrote:

@thockin https://github.com/thockin I suspect that might be another
side effect of the flag -> pflag transition from #3480
#3480...

I'll take a look, hopefully it's something easy to fix. For now I guess
--logtostderr=true should work.

Cheers,
Filipe

Reply to this email directly or view it on GitHub
#3740 (comment)
.

@filbranden
Copy link
Contributor Author

@thockin Yeah there's a problem there indeed...

This doesn't seem to happen with boolean flags registered with pflag itself (like --runonce) but --logtostderr is registered by the glog module which uses Go's native flag module so there might be something else going on there... I'll keep looking.

@filbranden
Copy link
Contributor Author

Ok, getting closer... I just found AddAllFlagsToPFlags() in pkg/util/pflag_import.go which is copying the flags from Go's flag to pflag, so a tweak there should fix this... I'll see if I can come up with a patch and I'll send you a PR for it shortly.

@filbranden filbranden mentioned this pull request Jan 23, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@filbranden @thockin @jbeda