chore: fix go lint findings #1079
Conversation
| @@ -46,7 +46,7 @@ type cli struct { | |||
| ListRulesFlag bool | |||
| } | |||
|
|
|||
| var ExitForLintFailure = errors.New("found problems during linting") | |||
There was a problem hiding this comment.
What was the warning for this? I'd prefer not to unexport this error. Technically it is a breaking change.
There was a problem hiding this comment.
it just needs a comment, so I'm happy to fix it.
What part of it is a breaking change? I checked and it wasn't referred to outside of the main package.
There was a problem hiding this comment.
What part of it is a breaking change? I checked and it wasn't referred to outside of the main package.
It is exported, so if anyone has depended upon this in their own code, it would break them. I don't really think it would be smart for someone to depend on something in a cmd/ directory, but it is possible if someone is "scripting" with Go code and handling the errors. If the only problem was missing a comment, let's not even risk annoying someone :)
| var ExitForLintFailure = errors.New("found problems during linting") | ||
| // ErrExistForLintFailure is an error to indicate that an error | ||
| // was found during linting. | ||
| var ErrExitForLintFailure = errors.New("found problems during linting") |
There was a problem hiding this comment.
Oh can you not change the name either please? That is breaking as well.
There was a problem hiding this comment.
this will conflict with the linter, so I guess we'll have to ignore this one.
I'm looking for the way to ignore this (// nolint:all isn't doing the trick even), but in the meantime, I just noticed that golint was deprecated: golangci/golangci-lint#1892 and staticheck is recommended as a replacement.
Should I do that work here? My only motivation for doing it now is that nolint isn't working for me.
There was a problem hiding this comment.
I made #1084 which has everything working with staticcheck instead. or I could merge this one in (probably will not be able to fix this one) and do staticcheck after. WDYT?
There was a problem hiding this comment.
Let's just go with your staticcheck PR, if all of the changes are the same anyways. Good call on switching, thank you.
Go lint had multiple findings when run on the full codebase. Fixing those to help ensure future contributors have less friction and confusion wondering if the errors were caused by them.
|
Closing in favor of #1084. |
Go lint had multiple findings when run on the full codebase.
Fixing those to help ensure future contributors have less friction and confusion wondering if the errors were caused by them.