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.

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

crictl-exec: fix argument parsing #306

Merged
merged 2 commits into from
May 8, 2018
Merged

crictl-exec: fix argument parsing #306

merged 2 commits into from
May 8, 2018

Conversation

vrothberg
Copy link
Contributor

@vrothberg vrothberg commented May 7, 2018

Commits:

  • crictl-exec: fix argument parsing
  • crictl: allow short option handling

Signed-off-by: Valentin Rothberg vrothberg@suse.com
Fixes: #305

@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


Instructions 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. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 7, 2018
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels May 7, 2018
@vrothberg
Copy link
Contributor Author

vrothberg commented May 7, 2018

Given 970fcf5, I will break it up into multiple commits to also add short opt handling for the remaining commands. The SkipArgReorder flag is likely to fix the issue.

@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 7, 2018
A command's arguments have been interpreted as flags leading to various
kinds of errors.  Fix it by vendoring the latest urfave/cli and setting
the corresponding cli.Command flag to avoid reordering of arguments.

Signed-off-by: Valentin Rothberg <vrothberg@suse.com>
Fixes: #305
Allow handling of short options (e.g., `exec -it` vs `exec -i -t).

Signed-off-by: Valentin Rothberg <vrothberg@suse.com>
@vrothberg
Copy link
Contributor Author

@runcom, PTAL. Both commits seem to fix the issues you have seen in 970fcf5.

Copy link

@flavio flavio left a comment

Choose a reason for hiding this comment

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

LGTM

@feiskyer
Copy link
Member

feiskyer commented May 8, 2018

@vrothberg Thanks for the fix

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 8, 2018
@feiskyer feiskyer merged commit ed19775 into kubernetes-sigs:master May 8, 2018
@vrothberg vrothberg deleted the fix-args-parsing branch May 8, 2018 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

crictl exec: cannot parse command args
4 participants