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.

Sign up for GitHub

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

Jump to bottom

UPSTREAM: 35206 update default run func for cmds containing sub-commands #11362

Merged
merged 2 commits into from
Nov 3, 2016
Merged

UPSTREAM: 35206 update default run func for cmds containing sub-commands #11362

merged 2 commits into from
Nov 3, 2016

Conversation

juanvallejo
Copy link
Contributor

@juanvallejo juanvallejo commented Oct 13, 2016 •
edited
Loading

UPSTREAM: kubernetes/kubernetes#35206

Fixes #11043 (comment)

This patch updates parent commands of sub-commands to exit with a usage
error and exit code 1 on an invalid (non-sub-command) argument.

Example

$ oc rollout
... [help output] ...

$ echo $?
0

$ oc rollout invalidsubcommand
error: unknown command "lol"
See 'oc rollout -h' for help and examples.

$ echo $?
1

cc @openshift/cli-review @Kargakis @tnozicka

@juanvallejo
Copy link
Contributor Author

[test]

@stevekuznetsov
Copy link
Contributor

Isn't usage error usually 2? Does cobra have return value constants somewhere?

@juanvallejo
Copy link
Contributor Author

juanvallejo commented Oct 14, 2016 •
edited
Loading

@stevekuznetsov

Isn't usage error usually 2?

From a few cases I've seen, UsageErrors have been returning 1 (e.g. https://github.com/openshift/origin/blob/master/pkg/cmd/cli/cmd/projects.go#L69)

$ oc projects extraneous_argument
error: no arguments should be passed
See 'oc projects -h' for help and examples.

$ echo $?
1

Does cobra have return value constants somewhere?

These are the only constants I've found in Cobra

@juanvallejo juanvallejo force-pushed the jvallejo/return-non-0-exit-code-non-existent-sub-cmds branch from 85cdd34 to 9eca98c Compare October 18, 2016 13:41
@juanvallejo
Copy link
Contributor Author

conformance flaked on #11114 re[test]

@juanvallejo juanvallejo force-pushed the jvallejo/return-non-0-exit-code-non-existent-sub-cmds branch 3 times, most recently from 0b0f5f2 to 390f3a1 Compare October 19, 2016 15:50
@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 19, 2016
@0xmichalis
Copy link
Contributor

No need for the upstream commits to be separate. Have you opened a PR in Kube already?

@juanvallejo juanvallejo force-pushed the jvallejo/return-non-0-exit-code-non-existent-sub-cmds branch from 390f3a1 to 24d51bf Compare October 20, 2016 14:24
@juanvallejo
Copy link
Contributor Author

@Kargakis

No need for the upstream commits to be separate. Have you opened a PR in Kube already?

Done! kubernetes/kubernetes#35206

@juanvallejo juanvallejo changed the title update default run func for cmds containing sub-commands UPSTREAM: 35206 update default run func for cmds containing sub-commands Oct 20, 2016
@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2016
@juanvallejo juanvallejo force-pushed the jvallejo/return-non-0-exit-code-non-existent-sub-cmds branch 2 times, most recently from 5bd7640 to cf5bc95 Compare October 24, 2016 19:17
@juanvallejo
Copy link
Contributor Author

networking test flaked on #11567 re[test]

@juanvallejo juanvallejo force-pushed the jvallejo/return-non-0-exit-code-non-existent-sub-cmds branch from cf5bc95 to 57cde07 Compare November 2, 2016 19:59
@0xmichalis
Copy link
Contributor

Can you update the commit name of the UPSTREAM commit with the real PR #

@juanvallejo
UPSTREAM: 35206: update default subcommand run func
39c1f2e 
This patch updates parent commands of sub-commands to exit with a usage
error and exit code 1 on an invalid (non-sub-command) argument. It also
upstreams the DefaultSubCommandRun util.
@juanvallejo
update default subcommand run func
e88bf66 
This patch updates parent commands of sub-commands to exit with a usage
error and exit code 1 on an invalid (non-sub-command) argument. It also
upstreams the DefaultSubCommandRun util.
@juanvallejo juanvallejo force-pushed the jvallejo/return-non-0-exit-code-non-existent-sub-cmds branch from 57cde07 to e88bf66 Compare November 3, 2016 14:29
@juanvallejo
Copy link
Contributor Author

@Kargakis

Can you update the commit name of the UPSTREAM commit with the real PR #

Done!

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to e88bf66

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/11068/) (Base Commit: 3b6c372)

@fabianofranz
Copy link
Member

[merge]

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 3, 2016 •
edited
Loading

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/11068/) (Image: devenv-rhel7_5309)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to e88bf66

@openshift-bot openshift-bot merged commit 14e40d3 into openshift:master Nov 3, 2016
@juanvallejo juanvallejo deleted the jvallejo/return-non-0-exit-code-non-existent-sub-cmds branch November 4, 2016 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@juanvallejo @stevekuznetsov @0xmichalis @openshift-bot @fabianofranz