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

Set SilenceErrors: true on rootCmd so that we can deal with errors ourselves #96

Merged
merged 1 commit into from
May 10, 2019
Merged

Set SilenceErrors: true on rootCmd so that we can deal with errors ourselves #96

merged 1 commit into from
May 10, 2019

Conversation

maximilien
Copy link
Contributor

Fixes issue #89

@knative-prow-robot knative-prow-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 8, 2019
@maximilien maximilien changed the title Remove fmt.FPrintln of error since creates double error output to the user Remove fmt.Fprintln of error since creates double error output to the user May 8, 2019
@cppforlife
Copy link

where is the printing coming from? i think we should keep consistent top level error printing and remove dup from somewhere else.

@maximilien
Copy link
Contributor Author

Actually the issue is that Cobra is automatically dealing with errors by default. See latest commit. I silence errors from Cobra and deal with them globally again in main.go.

@maximilien maximilien changed the title Remove fmt.Fprintln of error since creates double error output to the user Set SilenceErrors: true on rootCmd so that we can deal with errors ourselves May 9, 2019
@maximilien
Copy link
Contributor Author

Tests failures seem to come from tests servers giving 404 for Istio. See sample output:

I0509 21:00:41.340] error: unable to read URL "https://storage.googleapis.com/knative-nightly/serving/latest/istio-crds.yaml", server reported 404 Not Found, status code=404
I0509 21:00:41.346] ERROR: Knative setup failed

@maximilien
Copy link
Contributor Author

/retest

@cppforlife
Copy link

/lgtm
/approve

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label May 10, 2019
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cppforlife, maximilien, 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 10, 2019
@knative-prow-robot knative-prow-robot merged commit 47a10c0 into knative:master May 10, 2019
maximilien added a commit to maximilien/client that referenced this pull request May 14, 2019
@maximilien
Remove fmt.Printf of error since creates double error print to user (k…
1a4e740 
…native#96)

Fixes issue knative#89
maximilien added a commit to maximilien/client that referenced this pull request May 14, 2019
@maximilien
Remove fmt.Printf of error since creates double error print to user (k…
5449606 
…native#96)

Fixes issue knative#89
maximilien added a commit to maximilien/client that referenced this pull request May 14, 2019
@maximilien
Remove fmt.Printf of error since creates double error print to user (k…
9b2095b 
…native#96)

Fixes issue knative#89
@navidshaikh navidshaikh mentioned this pull request May 20, 2019
Closed
@maximilien maximilien deleted the issue89 branch December 20, 2019 02:18
coryrc pushed a commit to coryrc/client that referenced this pull request May 14, 2020
@adrcunha @knative-prow-robot
Create a Makefile to deploy Gubernator (knative#124)
b150f1d 
* Create a Makefile to deploy Gubernator

Bonus: fixes knative#74.

* Also fix knative#96.

* Add copyright boilerplate to redir_github.py
dsimansk added a commit to dsimansk/client that referenced this pull request Mar 3, 2023
@dsimansk
[release-v1.7] Update mod and xterm libraries to latest (knative#96)
e53f998
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@navidshaikh navidshaikh navidshaikh approved these changes

@cppforlife cppforlife Awaiting requested review from cppforlife

@sixolet sixolet Awaiting requested review from sixolet

Assignees

@cppforlife cppforlife

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@maximilien @cppforlife @knative-prow-robot @navidshaikh