-
Notifications
You must be signed in to change notification settings - Fork 8
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
Align Makefile - Goimports instead of go fmt #19
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: razo7 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Makefile
Outdated
fmt: ## Run go fmt against code. | ||
go fmt ./... | ||
fmt: goimports ## Run go goimports against code - goimports = go fmt + fixing imports. | ||
$(GOIMPORTS) -w ./main.go ./api ./controllers ./test |
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.
./pkg
is missing. Also there is no ./test
in this repo (yet), not sure if goimports comlains about it?
Copy / paste is ok, but always try to understand what the copied code is doing, and if it works in your own project 🙂
Makefile
Outdated
vet: ## Run go vet against code. | ||
go vet ./... | ||
vet: ## Run go vet against code - report likely mistakes in packages. | ||
go vet ./api/... ./controllers/... ./test/... |
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.
pkg and main.go is missing
Goimports = go fmt + fixing imports
|
||
.PHONY: vet | ||
vet: ## Run go vet against code. | ||
vet: ## Run go vet against code - report likely mistakes in packages. | ||
go vet ./... |
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.
ooooohhhh.... this is new! Well, for me at least 😎. ./...
used to also match the vendor
dir, that's why we started listing our files/dirs manually. Great to see that it doesn't anymore. Well, since Go 1.9 already?! 🙈 Just to be sure, goimports
is still trying to modify files in vendor
?
https://go.dev/doc/go1.9#vendor-dotdotdot
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.
goimports
v0.6.0 didn't work with ./...
, so unless we add ./vendor
then goimports won't modify the files under vendor.
/lgtm One question about goimports inside, feel free to unhold if |
/unhold |
In the process of moving FAR to Medik8s organization we align the Makefile - sixth PR.
Use Goimports instead of
go fmt
since Goimports equals to runninggo fmt
and fixing imports.