-
Notifications
You must be signed in to change notification settings - Fork 159
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
ci: add golang lint and fixed. #157 #320
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.
Thanks for working on this, @u5surf! For starters, it's probably fine to go with the default linting as here: we can always add additional rules later 👍
I have a few questions and small CRs - please check the comments.
One more thing: can you please add a small instruction / command on how to run this linter locally? It could be a target in Makefile or part of internal docs (i.e. |
Makefile
Outdated
lint: | ||
go install github.com/golangci/golangci-lint/cmd/[email protected] | ||
golangci-lint --timeout 30m run ./... |
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.
Nice 🙌 Timeout is pretty large: our code base is rather small so shouldn't need that much in theory 🤔 Either way, this would work, thanks for adding Makefile 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.
@yorugac
I could reduce the timeout to 5min.
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.
👍 thanks. It should be fine with 5m, and if not, we can always tinker with it.
* use the latest version of golangci-lint in github actions * add the target of lint in Makefile * fix nolint errcheck
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! Thank you @u5surf, for working on this 😄
Fixes #157
Add golangci-lint on github actions ci and fixed some conventional issues of linting.