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.

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

Update cobra error handling #58

Merged
merged 2 commits into from Aug 14, 2019

Conversation

@cblecker
Copy link
Contributor

commented Aug 13, 2019

follow up to #52 (comment)

@cblecker cblecker force-pushed the cblecker:error-handling branch from 9eb5cc9 to aa6e9fe Aug 13, 2019

@jhernand
Copy link
Collaborator

left a comment

Very nice and comprehensive change @cblecker. Thank you very much. Looks good to me, except a couple of places where I think we need to preserve the \n in the fmt.Printf(...) calls. Check the comments inline. Once you address that I will merge.

}

// Display quota information:
fmt.Printf("Cluster quota for organization '%s' ID: '%s'\n",
fmt.Printf("Cluster quota for organization '%s' ID: '%s'",

This comment has been minimized.

Copy link
@jhernand

jhernand Aug 14, 2019

Collaborator

I think that here we need to preserve the \n because this is not returned as an error, just printed.

This comment has been minimized.

Copy link
@cblecker

cblecker Aug 14, 2019

Author Contributor

Fixed!

Show resolved Hide resolved cmd/uhc/token/cmd.go Outdated

@cblecker cblecker force-pushed the cblecker:error-handling branch from aa6e9fe to a70159b Aug 14, 2019

@jhernand

This comment has been minimized.

Copy link
Collaborator

commented Aug 14, 2019

[test]

@jhernand

This comment has been minimized.

Copy link
Collaborator

commented Aug 14, 2019

Looks like there is some problem with the Jenkins instance that runs the tests. I am merging anyhow.

@jhernand jhernand merged commit 48fcf01 into openshift-online:master Aug 14, 2019

1 check was pending

ci.ext.devshift.net PR build Build triggered for merge commit.
Details

@cblecker cblecker deleted the cblecker:error-handling branch Aug 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.