Closed rajansandeep closed 4 years ago
We should consider ignoring vendor dirs in the context of go fmt
.
@justinsb mentions some of the benefits of keeping the vendor/ in the installer pkg and it would be a conscious decision to remove it that is out of scope for this patch.
This LGTM, but we need to exclude vendor. It might be easiest as @njhale suggests to put the verification steps into a script / makefile target that the github CI then calls; that makes for faster iteration and a better dev experience.
Searching around the web, the easiest way to ignore the vendor directory is likely to do this:
goimports -d $(find . -type f -name '*.go' -not -path "*/vendor/*")
(the go list
alternative approach does not work with nested go modules, AFAICT)
WDYT @rajansandeep ?
Searching around the web, the easiest way to ignore the vendor directory is likely to do this:
goimports -d $(find . -type f -name '*.go' -not -path "*/vendor/*")
(the
go list
alternative approach does not work with nested go modules, AFAICT)WDYT @rajansandeep ?
@justinsb Yes, Thank you! I was on the same track except that the command you suggested didn't work for me somehow... maybe I had a typo... Anyways, now the vendor dir is excluded from the goimport checks.
Thanks @rajansandeep - looks great!
/approve /lgtm
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: justinsb, rajansandeep
The full list of commands accepted by this bot can be found here.
The pull request process is described here
Adds workflow and also cleans up the test failures