-
Notifications
You must be signed in to change notification settings - Fork 527
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
Enable some more golangci-lint checks, fix findings #1164
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1164 +/- ##
=======================================
Coverage 75.22% 75.22%
=======================================
Files 108 108
Lines 7850 7850
=======================================
Hits 5905 5905
Misses 1379 1379
Partials 566 566
Continue to review full report at Codecov.
|
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.
I stopped commenting on them, but there were a bunch of // nolint errorlint
that could be errors.As
, and a license presubmit check, but otherwise this LGTM
070d27b
to
588c42a
Compare
There's still quite a bit of errchecks to chase down, but this is an improvement at least.
c4b24b7
to
f889c58
Compare
_, ok := err.(*ErrBadName) | ||
return ok | ||
var berr *ErrBadName | ||
return errors.As(err, &berr) |
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.
I believe this can just be errors.Is
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.
I tried a bunch of different ways and couldn't get it to work.
It turns out this method is never used outside of tests in pkg/name
, so I just removed it and inlined it (with errors.As
for now) into the two places in tests where it was used. In theory someone is using it today, since it was exported. If we want I can put it back and deprecate it.
I also removed NewErrBadName
which isn't used outside of pkg/name
, and should not be needed outside of pkg/name
.
There's still quite a bit of errchecks to chase down, but this is an
improvement at least.
god what a mind-numbing chore.