-
Notifications
You must be signed in to change notification settings - Fork 135
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
Introduce more stringent golangci-lint checks & address issues raised #538
Conversation
Signed-off-by: Thomas Stromberg <[email protected]>
Signed-off-by: Thomas Stromberg <[email protected]>
Signed-off-by: Thomas Stromberg <[email protected]>
Signed-off-by: Thomas Stromberg <[email protected]>
Signed-off-by: Thomas Stromberg <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #538 +/- ##
==========================================
+ Coverage 34.36% 34.76% +0.39%
==========================================
Files 44 44
Lines 3384 3348 -36
==========================================
+ Hits 1163 1164 +1
+ Misses 2122 2085 -37
Partials 99 99
Continue to review full report at Codecov.
|
FWIW, I can't make heads or tails about the CI check failure:
When I run |
@tstromberg . I see this PR has conflicts. Please can you update PR? |
Signed-off-by: Thomas Stromberg <[email protected]>
…ge more easily Signed-off-by: Thomas Stromberg <[email protected]>
Signed-off-by: Thomas Stromberg <[email protected]>
Signed-off-by: Thomas Stromberg <[email protected]>
Signed-off-by: Thomas Stromberg <[email protected]>
It appears that This PR is now ready for review. |
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
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 left some questions and nitpics but all in all this looks great.
One more non-blocking question - are the changes to the generated files (mocks and protobufs) going to cause more PR noise on subsequent PRs or will our make
commands ensure that the generated files are formatted properly?
Signed-off-by: Thomas Stromberg <[email protected]>
cd512b9
That's a great question: the short answer is that regenerating the protobuf related files may cause the |
Signed-off-by: Thomas Stromberg <[email protected]>
Description
This introduces a more stringent
golangci-lint
configuration from thelint-install
tool, as well as adds some Makefile rules to run lint locally in a way that matches CI.Detailed Changes
This PR includes fixes for the following lint changes, as well as some basic reformatting (
gofumpt
for Go, andprettier
for YAML). This PR does not enforce the Dockerfile lint checks yet, but does warn about them.This PR does add some
//nolint
directives for issues that require substantial future refactoring work.Why is this needed
Fixes #512
How Has This Been Tested?
make test