-
Notifications
You must be signed in to change notification settings - Fork 355
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
Add a new lint
task to replace the lint-go
task and use golangci-lint
#679
Conversation
faa8751
to
7e561c5
Compare
e199b4f
to
f7ef820
Compare
9f8a935
to
19a80e1
Compare
f7ef820
to
9996125
Compare
19a80e1
to
3cbb3d4
Compare
lint
task to replace the lint-go
task and use golangci-lint
a3c4aa9
to
efe13c0
Compare
8f7bb69
to
df326d0
Compare
df326d0
to
0155250
Compare
Since this is a big change, I will not merge this until after our next release. |
0155250
to
3a26d2e
Compare
This will eventually replace the existing `lint-go` and `lint-js` tasks
This includes fixes for a lot of linter issues. The vast majority of these were: * Unused functions, vars, and arguments * Missing error checking (I added a lot of `//nolint` comments where it was too painful to add actual checking). * Passing `nil` as a context * Using the deprecated `ioutil` package
There are a huge number of these because of the Go driver upgrade. There is a separate ticket, TOOLS-3633, for fixing these.
f6d7e35
to
dd36c5a
Compare
// errors are received | ||
func channelQuorumError(ch <-chan error, quorum int) (err error) { | ||
// errors are received. | ||
func channelQuorumError(ch <-chan error) (err error) { |
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.
Why does this function need to change?
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.
Every single caller set quorum
to 2
, so I just made this a const
.
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.
It seems that the function's comment wasn't updated.
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.
Updated.
Co-authored-by: Jian Guan <[email protected]>
Co-authored-by: Jian Guan <[email protected]>
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 % one remaining comment.
// errors are received | ||
func channelQuorumError(ch <-chan error, quorum int) (err error) { | ||
// errors are received. | ||
func channelQuorumError(ch <-chan error) (err error) { |
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.
It seems that the function's comment wasn't updated.
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 % one question
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.
Might've missed it, but are we using this anywhere? (if not, do we think we'll use it?)
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.
Oops, we will use it in a follow-up PR. Not sure how that snuck into this one, but I'll leave it since the next PR is just waiting on this one to be merged.
This gets rid of the ancient fork of
golint
we were using and replaces it withgolangci-lint
, fixing quite a few linting issues found bygolangci-lint
.It also removes the
format-go
task, as that can be done withprecious
andgolangci-lint
as well.