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 and github.com/spf13/cobra #27855

Merged
merged 1 commit into from
Aug 12, 2016
Merged

Update github.com/spf13/pflag and github.com/spf13/cobra #27855

merged 1 commit into from
Aug 12, 2016

Conversation

andreykurilin
Copy link
Contributor

@andreykurilin andreykurilin commented Jun 22, 2016 •
edited by k8s-oncall
Loading

Update github.com/spf13/pflag and github.com/spf13/cobra

Update:
github.com/spf13/cobra to f62e98d28ab7ad31d707ba837a966378465c7b57
github.com/spf13/cobra/doc to f62e98d28ab7ad31d707ba837a966378465c7b57
github.com/spf13/pflag to 1560c1005499d61b80f865c04d39ca7505bf7f0b

Closes issue #29852

This change is Reviewable

@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-label-needed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 22, 2016
@eparis eparis added lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Jun 23, 2016
@eparis eparis self-assigned this Jun 23, 2016
@eparis eparis added this to the v1.4 milestone Jun 23, 2016
@k8s-github-robot k8s-github-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 23, 2016
@eparis
Copy link
Contributor

eparis commented Jun 24, 2016

ok to test

@eparis eparis added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 24, 2016
@andreykurilin
Copy link
Contributor Author

@eparis Should I close this pr, since issue was marked as duplicate and it is closed now or it can be merged just like for updating code?

@elyscape
Copy link
Contributor

elyscape commented Jul 6, 2016 •
edited
Loading

IMO, it's still a good idea to update cobra, because that would pull in spf13/cobra#288, which fixes the bash completion for kubectl create as described in spf13/cobra#287. In short, it will make kubectl create <TAB> suggest configmap, namespace, secret, and serviceaccount in addition to -f and --filename=.

@k8s-github-robot k8s-github-robot added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. retest-not-required-docs-only and removed retest-not-required-docs-only labels Jul 28, 2016
@wojtek-t wojtek-t removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Jul 31, 2016
@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 1, 2016
@eparis
Copy link
Contributor

eparis commented Aug 5, 2016

@andreykurilin looks like there was a slight change in the cobra error message which the _test.go file complains about. Likely we need to update an _test.go as well?

@andreykurilin
Copy link
Contributor Author

@eparis yeah, I saw it, but had no time to fix it. I'll try to do it at the beginning of next week

@andreykurilin
Update github.com/spf13/pflag and github.com/spf13/cobra
1b00a2d 
Update:
  github.com/spf13/cobra to f62e98d28ab7ad31d707ba837a966378465c7b57
  github.com/spf13/cobra/doc to f62e98d28ab7ad31d707ba837a966378465c7b57
  github.com/spf13/pflag to 1560c1005499d61b80f865c04d39ca7505bf7f0b

Closes issue #29852
@k8s-bot
Copy link

k8s-bot commented Aug 9, 2016

GCE e2e build/test passed for commit 1b00a2d.

@andreykurilin
Copy link
Contributor Author

@eparis, tests are green now

@andreykurilin andreykurilin mentioned this pull request Aug 10, 2016
Closed
adohe-zz
adohe-zz reviewed Aug 11, 2016
View reviewed changes
cmd/hyperkube/hyperkube_test.go
assert.NotContains(t, x.output, "test1 Run")
}

func TestServerFlagsBad(t *testing.T) {
x := runFull(t, "hyperkube test1 --bad-flag")
assert.EqualError(t, x.err, "unknown flag: --bad-flag")
assert.Contains(t, x.output, "A simple server named test1")
assert.Contains(t, x.output, "--help[=false]: help for hyperkube")
assert.Contains(t, x.output, "-h, --help help for hyperkube")
assert.NotContains(t, x.output, "test1 Run")
Copy link

Choose a reason for hiding this comment

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

This looks like a little bit strange, the help output really contains so many extra spaces?

Copy link
Contributor Author

@andreykurilin andreykurilin Aug 11, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Yes. Here you can find example(several options) of old help:

      --global-housekeeping-interval=1m0s: Interval between global housekeepings
      --google-json-key="": The Google Cloud Platform Service Account JSON Key to use for authentication.
      --healthz-bind-address=127.0.0.1: The IP address for the health check server to serve on, defaulting to 127.0.0.1 (set to 0.0.0.0 for all interfaces)
      --healthz-port=10249: The port to bind the health check server. Use 0 to disable.
  -h, --help[=false]: help for hyperkube
      --hostname-override="": If non-empty, will use this string as identification instead of the actual hostname.
      --housekeeping-interval=10s: Interval between container housekeepings
      --iptables-masquerade-bit=14: If using the pure iptables proxy, the bit of the fwmark space to mark packets requiring SNAT with.  Must be within the range [0, 31].
      --iptables-sync-period=30s: How often iptables rules are refreshed (e.g. '5s', '1m', '2h22m').  Must be greater than 0.

after this patch it will look like:

      --global-housekeeping-interval value                 Interval between global housekeepings (default 1m0s)
      --google-json-key string                             The Google Cloud Platform Service Account JSON Key to use for authentication.
      --healthz-bind-address value                         The IP address for the health check server to serve on, defaulting to 127.0.0.1 (set to 0.0.0.0 for all interfaces) (default 127.0.0.1)
      --healthz-port value                                 The port to bind the health check server. Use 0 to disable. (default 10249)
  -h, --help                                               help for hyperkube
      --hostname-override string                           If non-empty, will use this string as identification instead of the actual hostname.
      --housekeeping-interval value                        Interval between container housekeepings (default 10s)
      --iptables-masquerade-bit value                      If using the pure iptables proxy, the bit of the fwmark space to mark packets requiring SNAT with.  Must be within the range [0, 31]. (default 14)
      --iptables-sync-period duration                      How often iptables rules are refreshed (e.g. '5s', '1m', '2h22m').  Must be greater than 0. (default 30s)

Copy link

Choose a reason for hiding this comment

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

Nice, this looks more user friendly. Thanks @andreykurilin

@adohe-zz
Copy link

LGTM.
@eparis kindly ping, please help add the label.

@eparis
Copy link
Contributor

eparis commented Aug 11, 2016

:lgtm:

Review status: 0 of 15 files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable

@eparis
Copy link
Contributor

eparis commented Aug 11, 2016

Reviewed 15 of 15 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable

@eparis eparis added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 11, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Aug 12, 2016

GCE e2e build/test passed for commit 1b00a2d.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit ca92a20 into kubernetes:master Aug 12, 2016
@andreykurilin andreykurilin deleted the cobra_update branch August 12, 2016 06:41
@andreykurilin
Copy link
Contributor Author

@eparis @adohe thanks!

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

@eparis eparis

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
v1.4
Development

Successfully merging this pull request may close these issues.
8 participants
@andreykurilin @eparis @elyscape @k8s-bot @adohe-zz @k8s-github-robot @googlebot @wojtek-t