-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Refactor parsing to not require --remote to be first flag #7212
Conversation
LGTM |
/lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a test to prevent regressing?
It's okay if we have non-zero exit status but check for the correct error message.
e6bb27a
to
34cd6d3
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jwhonce The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Use cobra.Command.FParseErrWhitelist to no longer require --remote to be the first argument in flags when using CLI Signed-off-by: Jhon Honce <jhonce@redhat.com>
LGTM. @vrothberg I agree with the need for a test, but this is hard to test because it requires muckery (detecting that the podman command is |
/hold cancel |
@edsantiago Thanks! /hold cancel |
/lgtm |
- new sanity checks for podman-remote: - first, confirm that when PODMAN is "-remote", we actually talk to a server (validated by presence of "Server:" string in "podman version"). - second, add test for containers#7212, in which we run "podman --remote" (podman with --remote flag, not podman-remote command) and make sure --remote is allowed both as the first option and also with other flag options preceding. - new test for "podman image tree" (piggybacking on top of a "podman build" test, because that gives us lots of layers). - skip "podman exec - basic test" when remote. It is consistently causing CI failures, breaking all of CI, due to containers#7241. Signed-off-by: Ed Santiago <santiago@redhat.com>
- new sanity checks for podman-remote: - first, confirm that when PODMAN is "-remote", we actually talk to a server (validated by presence of "Server:" string in "podman version"). - second, add test for containers#7212, in which we run "podman --remote" (podman with --remote flag, not podman-remote command) and make sure --remote is allowed both as the first option and also with other flag options preceding. - new test for "podman image tree" (piggybacking on top of a "podman build" test, because that gives us lots of layers). - skip "podman exec - basic test" when remote. It is consistently causing CI failures, breaking all of CI, due to containers#7241. Signed-off-by: Ed Santiago <santiago@redhat.com>
Use cobra.Command.FParseErrWhitelist to no longer require --remote to be
the first argument in flags when using CLI
Fixes: #7211
Signed-off-by: Jhon Honce jhonce@redhat.com