Urfave cli v2 #571
Merged
Urfave cli v2 #571
Conversation
This file contains 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
purpleidea
reviewed
Nov 12, 2019
lang/gapi.go
Outdated
| Name: flagModulePath, | ||
| Value: "", // empty by default | ||
| Usage: "choose the modules path (absolute)", | ||
| EnvVars: []string("MGMT_MODULE_PATH"), |
There was a problem hiding this comment.
I bet you meant:
[]string{"MGMT_MODULE_PATH"},
|
You might also need to update something here: https://github.com/purpleidea/mgmt/blob/master/gapi/gapi.go#L104 |
ffrank
force-pushed
the
urfave-cli-v2
branch
from
06dbf3d to
96b1886
Compare
purpleidea
reviewed
Nov 12, 2019
| @@ -41,207 +41,207 @@ func CLI(program, version string, flags Flags) error { | |||
| } | |||
|
|
|||
| // All of these flags can be accessed in your GAPI implementation with | |||
| // the `c.Parent().Type` and `c.Parent().IsSet` functions. Their own | |||
| // the `c.Lineage()[1].Type` and `c.Lineage()[1].IsSet` functions. Their own | |||
There was a problem hiding this comment.
It's especially dope when you also port the comments. I do this too. Not being sarcastic. 👍
ffrank
force-pushed
the
urfave-cli-v2
branch
from
96b1886 to
34068b6
Compare
purpleidea
requested changes
Nov 12, 2019
lang/gapi.go
Outdated
| // Name: "stdin", | ||
| // Usage: "use passthrough stdin", | ||
| //}, | ||
| modulePath, | ||
| &modulePath, |
There was a problem hiding this comment.
actually I think you want the & up higher in the modulePath definition.
eg: modulePath := cli.StringFlag{ becomes:
modulePath := &cli.StringFlag{
There was a problem hiding this comment.
Ah. Yes, this is silly. It was one of the first places I touched and had not yet grokked this.
ffrank
force-pushed
the
urfave-cli-v2
branch
from
34068b6 to
33d89c2
Compare
|
LGTM =D ... Waiting for travis. PS: Did you smoke test to see if the nested GAPI stuff works? |
|
Merged, thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.