Migrate Urfave CLI from v1 to v2#9809
Conversation
|
Hi @dereknola. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
dereknola
force-pushed
the
urfave_v2
branch
2 times, most recently
from
183a54f to
745644a
Compare
|
Please squash commits |
|
Squashed all commits |
|
/ok-to-test |
dmcgowan
left a comment
dmcgowan
left a comment
There was a problem hiding this comment.
One small concern on a string slice, otherwise looks good
|
Do I need to squash again? |
Yes. So that the changes are available as a single commit. |
|
/retest |
Followed the Migration Guide at https://cli.urfave.org/migrate-v1-to-v2/ The major changes not pointed out in the migration guide are: - context.Args() no longer produces a []slice, so context.Args().Slice() in substitued - All cli.Global***** are deprecated (the migration guide is somewhat unclear on this) Signed-off-by: Derek Nola <[email protected]> Vendor in urfave cli/v2 Signed-off-by: Derek Nola <[email protected]> Fix NewStringSlice calls Signed-off-by: Derek Nola <[email protected]>
estesp
dismissed
dmcgowan’s stale review
slice issue was resolved in last push
7 participants
Changes
Vendoring/go mod updates is in a separate commit for PR review clarity
Followed the Migration Guide at https://cli.urfave.org/migrate-v1-to-v2/
Some points from this guide:
EnvVaris nowEnvVars, a []stringThe major changes not pointed out in the migration guide are:
context.Args()no longer produces a []slice, socontext.Args().Slice()in substituted
cli.Global*****are deprecated (the migration guide is somewhat unclear on this)Verification:
All binaries build using
make.Linked Issue
#9808