Standardizes errors in commands #359
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
se7entyse7en
force-pushed
the
standardization-err-commands
branch
2 times, most recently
from
c640a26
to
0e2d0c0
Compare
|
Something failed in the integration tests |
carlosms
approved these changes
Mar 21, 2019
smacker
reviewed
Mar 21, 2019
cmd/srcd/cmd/parse.go
Outdated
| @@ -118,7 +118,7 @@ The remaining nodes are printed to standard output in JSON format.`, | |||
| Mode: mode, | |||
| }) | |||
| if err != nil { | |||
| return fmt.Errorf("%T %v", err, err) | |||
| return fmt.Errorf("%T %v", err, humanize(err)) | |||
There was a problem hiding this comment.
addressed here.
smacker
reviewed
Mar 21, 2019
cmd/srcd/cmd/components.go
Outdated
| @@ -43,7 +43,7 @@ var componentsListCmd = &cobra.Command{ | |||
|
|
|||
| cmps, err := components.List(context.Background(), allVersions) | |||
| if err != nil { | |||
| return fatal(err, "could not list images") | |||
| return wrapHumanizeErr(err, "could not list images") | |||
There was a problem hiding this comment.
nit-pick: because we use this function a lot I would prefer shorter name. Options from the top of my head:
humanizefwrapfuserErr
There was a problem hiding this comment.
I don't know maybe whumanizef? so that we have both humanize and whumanizef.
There was a problem hiding this comment.
whumanizef
Please don't 😄
There was a problem hiding this comment.
😂 I think that humanizef is enough.
There was a problem hiding this comment.
Addressed here.
se7entyse7en
force-pushed
the
standardization-err-commands
branch
from
0e2d0c0
to
bc9a725
Compare
|
Rebased and force pushed to include integration testing for OSX. Please take a look at last 3 commits, especially this. |
carlosms
reviewed
Mar 21, 2019
cmd/srcd/cmd/errors.go
Outdated
| // fatal converts known errors to human friendly message and logs it with fatal level | ||
| func fatal(err error, format string, args ...interface{}) { | ||
| msg := fmt.Sprintf(format, args...) | ||
| // wrapHumanizeErr wraps and converts known errors to human friendly message |
There was a problem hiding this comment.
wrong name after the function rename.
smacker
approved these changes
Mar 22, 2019
…ands Signed-off-by: Lou Marvin Caraig <loumarvincaraig@gmail.com>
Signed-off-by: Lou Marvin Caraig <loumarvincaraig@gmail.com>
… Run Signed-off-by: Lou Marvin Caraig <loumarvincaraig@gmail.com>
se7entyse7en
force-pushed
the
standardization-err-commands
branch
from
9d880dd
to
92e2350
Compare
Signed-off-by: Lou Marvin Caraig <loumarvincaraig@gmail.com>
Signed-off-by: Lou Marvin Caraig <loumarvincaraig@gmail.com>
se7entyse7en
force-pushed
the
standardization-err-commands
branch
from
92e2350
to
365312d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #291.
Closes #326.