-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Drop github.com/pkg/errors
and fix error_code
detection
#1951
Conversation
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.
LGTM, but I would prefer to get the tests from #1660 ported, before we merge this
0777398
to
f995bbb
Compare
Codecov Report
@@ Coverage Diff @@
## master #1951 +/- ##
==========================================
+ Coverage 71.26% 71.30% +0.04%
==========================================
Files 181 183 +2
Lines 14256 14274 +18
==========================================
+ Hits 10159 10178 +19
+ Misses 3478 3476 -2
- Partials 619 620 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
github.com/pkg/errors
dependency and fix some lint issuesgithub.aaakk.us.kg/pkg/errors
and fix error_code
detection
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.
Yes, I know, I deliberately didn't fix it as a part of the cherry-pick. I'll probably squash this PR when merging it in |
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.
LGTM, I like the more descriptive error messages. Just left a couple of tiny nitpicks.
In the spirit of spring cleaning and #1933 😅
This also makes some of the changes that #1660 did, though in a slightly different way, so we should probably try to port the rest of them.