Cleanup error usage #3386
Cleanup error usage #3386
Conversation
|
@cpuguy83 thanks for this! In general we like to pave vendor/ changes discretely, and in advance to avoid merge conflicts. But because you authored the PR that induced these conflicts we're probably 👍 here :) |
Signed-off-by: Brian Goff <[email protected]>
Instead of masking errors with `fmt.Errorf()`, uses `errors.Wrap` where appropriate. Incidentally this also fixes some formatting issues in certain error messages.
Codecov Report
@@ Coverage Diff @@
## master #3386 +/- ##
=======================================
Coverage 54.51% 54.51%
=======================================
Files 104 104
Lines 15787 15787
=======================================
Hits 8606 8606
Misses 6429 6429
Partials 752 752 |
|
Thanks! I did at least put the vendor change in a separate commit here (though I guess I did not on the last change). This is rebased. |
| } | ||
|
|
||
| if len(uc.resourceGroupName) == 0 { | ||
| cmd.Usage() | ||
| return fmt.Errorf("--resource-group must be specified") | ||
| return errors.New("--resource-group must be specified") |
There was a problem hiding this comment.
Just curious: for error messages that aren't synthesizing downstream err objects with add'l string message context, what is the value of using github.com/pkg/errors? I.e., what the benefit of errors.New() over fmt.Errorf()?
There was a problem hiding this comment.
errors.New() stores the stack information which can aid in debugging... e.g. you can fmt.Printf("+v") and see the full stack trace of this error.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cpuguy83, jackfrancis 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 |
What this PR does / why we need it:
Use github.com/pkg/errors for defining errors.
Instead of masking errors with
fmt.Errorf(), useserrors.Wrapwhere appropriate.Incidentally this also fixes some formatting issues in certain error messages.