Switch cli to github.com/spf13/cobra #429
Conversation
openshift-ci-robot
added
size/XXL
wking
left a comment
wking
left a comment
There was a problem hiding this comment.
Looks good. I left a few suggestion inline which you can take or leave as you like.
cmd/openshift-install/main.go
Outdated
There was a problem hiding this comment.
Can we move these to main()? It feels strange to have an init() in a command package.
cmd/openshift-install/targets.go
Outdated
There was a problem hiding this comment.
i think kubectl example is more useful if we have more complex commands. but i can add new<cmd> type funcs.
There was a problem hiding this comment.
No I just meant "not in an init()" ;).
cmd/openshift-install/main.go
Outdated
There was a problem hiding this comment.
This shoiuld probably move to version.go.
This allows us to use the already vendored cobra cli package, and drop gopkg.in/alecthomas/kingpin.v2. This also splits cli into 3 different command groups: targets, destroy and version. cobra allows us to add new subcommands and local flags without putting everything into single switch in main.go
abhinavdahiya
force-pushed
the
switch_cli
branch
from
2c0ecbe to
dc118f2
Compare
|
@wking updated to remove |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya, wking The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Once the cluster is up, the bootstrap stuff is just a waste of resources. With this commit we make a reasonable effort to wait for the bootstrap-complete event, and, if we see it, we tear down the bootstrap resources. The Kubernetes client initialization is based on [1] and the local wait.go is based on [2]. Both of those are, like this project, under the Apache 2.0 license. Once kubernetes/kubernetes#50102 lands, we can drop the local wait.go and use the upstream implementation. I've dropped the errors.Cause call from main.go, because it was dropping the wrapped context, which is useful context for debugging. That Cause call is from dc118f2 (cmd: switch to already vendored cobra, 2018-10-08, openshift#429), but that commit doesn't explicitly motivate the call. [1]: https://github.com/kubernetes/client-go/blob/v9.0.0/examples/out-of-cluster-client-configuration/main.go [2]: https://github.com/kubernetes/kubernetes/pull/50102/files
4 participants
This allows us to use the already vendored cobra cli package, and drop gopkg.in/alecthomas/kingpin.v2.
This also splits cli into 3 different command groups: targets, destroy and version.
cobra allows us to add new subcommands and local flags without putting everything into single switch in main.go
/cc @wking