Conversation
Signed-off-by: Kevin Lingerfelt <kl@buoyant.io>
Signed-off-by: Kevin Lingerfelt <kl@buoyant.io>
|
CI passed 🎉 -- this is now ready for review. |
siggy
left a comment
siggy
left a comment
There was a problem hiding this comment.
Awesome! Sweet how you used this PR to fix linting in the 2nd commit. 👍 🚢
Consider adding a note on running this in TEST.md, somewhere around mention of go vet ./....
| go install ./vendor/golang.org/x/lint/golint | ||
|
|
||
| # use `go list` to exclude packages in vendor, ignore uncommented warnings | ||
| out=$(go list ./... | xargs golint | grep -v 'should have comment') || true |
There was a problem hiding this comment.
An alternative to this would be to use golangci-lint run --disable-all -E golint - this ignores all comment related linter errors, and in the future we could enable even more linters by adding a linter config file. If golint is the only one needed, then this is perfect.
There was a problem hiding this comment.
Wow! I had no idea that the go linting rabbit hole goes so deep :)
I wouldn't be opposed to adding additional linting configurations going forward, but I think golint minus comments suffices for our use case right now. That said, I'd happily review a PR that improved upon it. Thanks for the suggestion!
Signed-off-by: Kevin Lingerfelt <kl@buoyant.io>
Commit 1: Enable lint check for comments Part of #217. Follow up from #1982 and #2018. A subsequent commit will fix the ci failure. Commit 2: Address all comment-related linter errors. This change addresses all comment-related linter errors by doing the following: - Add comments to exported symbols - Make some exported symbols private - Recommend via TODOs that some exported symbols should should move or be removed This PR does not: - Modify, move, or remove any code - Modify existing comments Signed-off-by: Andrew Seigner <siggy@buoyant.io>
4 participants
This branch starts running golint on every CI build. I'm expecting this PR to fail CI with 8 lint warning, which I'll clean up in a subsequent commit. This is a follow-up to #1982.