lint: integrate impi #322
lint: integrate impi #322
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/S
Codecov Report
@@ Coverage Diff @@
## master #322 +/- ##
======================================
Coverage 57.3% 57.3%
======================================
Files 19 19
Lines 904 904
======================================
Hits 518 518
Misses 335 335
Partials 51 51Continue to review full report at Codecov.
|
|
Thanks! |
| @@ -28,4 +29,19 @@ then | |||
| fi | |||
|
|
|||
| # configured by .golangci.yml | |||
| exec env GO111MODULE=on "$gopath/bin/golangci-lint" run | |||
There was a problem hiding this comment.
There has been a discussion that we want to limit the scope of env variables as much as possible. Could you therefore inline the GO111MODULE env variable where necessary?
There was a problem hiding this comment.
Do you have the link of this discussion so that I can read it too?
I will revert it to stick to that discussion conclusion.
There was a problem hiding this comment.
It's here #243 (comment).
hack/run-lint.sh
Outdated
| trap 'rm -rf -- "${tmp}"' EXIT | ||
| cd "${tmp}" | ||
| go mod init foo | ||
| go get github.com/pavius/impi/cmd/impi@c1cbdcb |
There was a problem hiding this comment.
Is there a particular reason why you want to pin the exact commit version? For me,
GOPATH=/tmp/impi GO111MODULE=off GOBIN="/home/me/go/bin" go get github.com/pavius/impi/cmd/impiworks just fine.
There was a problem hiding this comment.
yes, works fine but they can break the master.
I thought we shouldn't break our tool when this happens.
However, since it's a dev tool, we might tolerate it and actually it will be a reminder to update the linter.
There was a problem hiding this comment.
now installing from master.
| # install impi that ensures import grouping is done consistently | ||
| if ! [[ -x "${gopath}/bin/impi" ]] | ||
| then | ||
| echo >&2 'Installing impi' |
There was a problem hiding this comment.
This block should be moved into a function as for install_gox in hack/install-gox.sh.
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: corneliusweig, ferhatelmas 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 |
k8s-ci-robot
added
the
approved
impi is added as a part of linting. A follow up on #321.
impi: validates if import order is done consistently.