-
Notifications
You must be signed in to change notification settings - Fork 261
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
Remove Printing Err When Running Root Command #615
Remove Printing Err When Running Root Command #615
Conversation
Hi @danielhelfand. Thanks for your PR. I'm waiting for a knative 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. 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. |
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.
/ok-to-test
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danielhelfand, navidshaikh 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 |
35c3584
to
f517bd9
Compare
@@ -34,10 +34,12 @@ func main() { | |||
defer cleanup() | |||
rand.Seed(time.Now().UnixNano()) | |||
err = core.NewDefaultKnCommand().Execute() | |||
if err != nil { | |||
fmt.Fprintln(os.Stderr, err) | |||
if err.Error() == "subcommand is required" { | |||
os.Exit(1) |
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.
Sorry - didn't have time to try this myself (stuck in calls), but if we immediately exit w/o printing anything, what does the user see?
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.
I just want to make sure we're not exiting w/o some indication of what went wrong so the user can fix it.
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.
So, in the most recent push, the only Error()
ignored is subcommand is required
, which is occurring when you run kn
by itself. So running kn
will result in the following:
Manage your Knative building blocks:
* Serving: Manage your services and release new software to them.
* Eventing: Manage event subscriptions and channels. Connect up event sources.
Usage:
kn [command]
Available Commands:
completion Output shell completion code
help Help about any command
plugin Plugin command group
revision Revision command group
route Route command group
service Service command group
source Event source command group
trigger Trigger command group
version Prints the client version
Flags:
--config string kn config file (default is $HOME/.kn/config.yaml)
-h, --help help for kn
--kubeconfig string kubectl config file (default is $HOME/.kube/config)
--log-http log http traffic
--lookup-plugins look for kn plugins in $PATH
--plugins-dir string kn plugins directory (default "~/.kn/plugins")
Use "kn [command] --help" for more information about a command.
If the error is not subcommand is required
, this approach should still print the error as originally implemented.
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.
ok thanks.
f517bd9
to
eb855e7
Compare
eb855e7
to
de6d221
Compare
/retest |
/lgtm |
* Create presubmit jobs for knative/client Also enable tide for the new repo. * Add the missing coverage job/tabs This is a workaround for knative#615.
Fixes #612
Proposed Changes
kn
root commandRelease
Fix subcommand is required error from showing when running kn by itself