Skip to content

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

Merged
openshift-bot merged 2 commits intoopenshift:masterfrom
juanvallejo:jvallejo/return-non-0-exit-code-non-existent-sub-cmds
Nov 3, 2016
Merged

UPSTREAM: 35206 update default run func for cmds containing sub-commands#11362
openshift-bot merged 2 commits intoopenshift:masterfrom
juanvallejo:jvallejo/return-non-0-exit-code-non-existent-sub-cmds

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

juanvallejo commented Oct 13, 2016

[test]

@stevekuznetsov
Copy link
Contributor

stevekuznetsov commented Oct 13, 2016

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

juanvallejo commented Oct 18, 2016

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

0xmichalis commented Oct 19, 2016

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

juanvallejo commented Oct 20, 2016

@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

juanvallejo commented Oct 26, 2016

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

0xmichalis commented Nov 2, 2016

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

juanvallejo commented Nov 3, 2016

@Kargakis

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

Done!

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 3, 2016

Evaluated for origin test up to e88bf66

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 3, 2016

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

@fabianofranz
Copy link
Member

fabianofranz commented Nov 3, 2016

[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

openshift-bot commented Nov 3, 2016

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