-
Notifications
You must be signed in to change notification settings - Fork 98
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
Check if incoming code is gofmt-ed #817
Comments
ah, I've completely missed this https://github.com/k8gb-io/k8gb/blob/master/.golangci.yaml#L43 so it looks like it's on purpose. I wonder why though. also probably dupe of #775 |
@jkremser Goimports does everything that gofmt does. Additionally it adds missing imports and removes unreferenced imports , according to this article. I found one warning with enabled # .golangci.yaml
# ...
- gofmt
# ...
gofmt:
# gofmt -s
simplify: true The question is, if we must keep both - goimports + gofmt or goimports can be configured to replace |
I see, so somehow the I'd probably whitelisted the gofmt, they don't fight against each other and can be run both (also the output is not duplicated), wdyt? right now, it reports
which is kind of misleading should be (if gofmt is enabled)
|
wrt terratest |
closing as solved, we can reopen if linting brings troubles again |
Currently neither the
golangci-lint run
nor thegokart scan --globalsTainted --verbose
do catch some things thatgofmt -w -s .
does to code.We can do
gofmt -l .
that lists all the files that are not aligned w/ the gofmt (~something like a dry run) and check if the output is empty.Currently it's complaining about terrratest:
I've checked couple of those and it's my work, perhaps I will setup also some pre-commit hook w/ gofmt. We could have this also in the repo itself, but it might be controversial.
note:
gofmt has an option
-s
that can simplify the code and these wannabe modifications are not listed w/-l
, but these can be checked bygofmt -s -d .
(again if the output is empty, we are good)The text was updated successfully, but these errors were encountered: