This repository was archived by the owner on Jan 30, 2020. It is now read-only.
fleetctl: use cobra instead of cli #1581
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dongsupark
force-pushed
the
dongsu/fleetctl-cobra-cli
branch
2 times, most recently
from
0e446f2 to
e16d251
Compare
dongsupark
force-pushed
the
dongsu/fleetctl-cobra-cli
branch
5 times, most recently
from
9dfab96 to
a7509a5
Compare
fleetctl/help.go
Outdated
| if len(flag.Deprecated) > 0 || flag.Hidden { | ||
| return | ||
| } | ||
| format := "" |
Contributor
There was a problem hiding this comment.
really these should probably either write into the output buffer directly, or use another bytes buffer here to construct the formatting string
Author
There was a problem hiding this comment.
Hmm, I'm not sure why it's written like that, as I copied the help.go from rkt. I just thought this was a common pattern. Will try to fix it.
dongsupark
force-pushed
the
dongsu/fleetctl-cobra-cli
branch
from
a7509a5 to
1d478d9
Compare
Author
|
Updated.
|
dongsupark
force-pushed
the
dongsu/fleetctl-cobra-cli
branch
2 times, most recently
from
115aaef to
2f3b2e7
Compare
- rework option parsing - move away from (most) global variables (cAPI), keep currentCommand for now... - we need to pass c (the cli context) and cAPI (the client API) around Signed-off-by: Olaf Buddenhagen <[email protected]>
Add github.com/spf13/cobra and github.com/spf13/pflag.
Let's roll back to the original global variable cAPI, to keep the entire code as simple as possible. As this is a single command-line tool, it would be harmless to simply use a global variable.
Use Cobra (github.com/spf13/cobra) instead of cli (github.com/codegangsta/cli), for better cmdline user interface. * Create a wrapper runWrapper() to be used for cobra, to wrap around a normal run*() function into a prototype for cobra.Command.Run(). It also sets a global variable cAPI for running a normal command. * remove unnecessary code for codegangsta/cli from fleetctl.go. Suggested-by: Jonathan Boulle <[email protected]> Fixes: coreos#1453 Supersedes coreos#1570
As every fleetctl command uses cobra instead of codegangsta/cli, we need to update unit tests as well.
* Do not compare output of help messages, to avoid occasional failures. As cobra sometimes prints out a different help message, it causes functional test TestClientHelpFlag to fail. * Run "fleetctl version" instead of "fleetctl --version". * Run "fleetctl help" instead of "fleetctl" to get help message.
dongsupark
force-pushed
the
dongsu/fleetctl-cobra-cli
branch
from
2f3b2e7 to
235875e
Compare
Author
|
I managed to remove duplicated calls for setting cAPI in each command, by moving the call into runWrapper(), as @jonboulle suggested. |
Author
|
Merged #1581. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use Cobra (https://github.com/spf13/cobra) instead of cli (https://github.com/codegangsta/cli), for better cmdline user interface. Create a wrapper
runWrapper()to be used for cobra, to wrap around a normalrun*()function into a prototype forcobra.Command.Run(). It also sets a global variable cAPI for running a normal command. Also remove unnecessary code for codegangsta/cli from fleetctl.go.This PR consists of 6 patches.
"fleetctl: move to cli"was taken from Move to cli #1570, though I had to modify that to get it working with the current vendor structure."vendor github.com/spf13/{cobra,pflag}"and 3rd"fleetctl: do not pass around cAPI"are just preparations."fleetctl: convert cli to cobra"actually covers most changes. As cobra affects nearly every fleetctl command, this is rather a huge change."fleetctl: use cobra also for unit tests"and 6th"functional: run correct commands for fleetctl help"are just minor updates for unit tests as well as functional tests.Suggested-by: @jonboulle
Fixes: #1453
Supersedes #1570
/cc @reneengelhard @kayrus @tixxdz