makefile: Add whitespace check #39
Conversation
kubevirt-bot
added
dco-signoff: yes
oshoval
force-pushed
the
ws
branch
2 times, most recently
from
fb33d1a
to
be1b791
Compare
|
/lgtm |
| @@ -67,6 +67,12 @@ create-nodeport: | |||
| bump-kubevirtci: | |||
| ./hack/bump-kubevirtci.sh | |||
|
|
|||
| whitespace: | |||
There was a problem hiding this comment.
I find it redundant to use third party github action for the whutespace and on the same time to implemet the logic by ourselves. I would keep just the github action.
It would be even better if we have an action verifying make fmt was invoked. Comparing the existing code and the code after make fmt.
There was a problem hiding this comment.
we dont use 3rd party, the github calls the target of make file
unless you mean make fmt or something fixes whitespace
github actions runs the check make whitespace-check
the github shows if there is an error, and then you can run locally easily the make whitespace in order to fix it easier, if forgotten
about fmt / vet it can be done later, not related to this PR
There was a problem hiding this comment.
you're right. I missed that, thought we are using a third party action.
Regardung the fmt- youre writing - Please remove trailing whitespaces using make fmt:' did you mean make whitespace? If make fmtdoes the job, butmake whitespace` is needed?
There was a problem hiding this comment.
should be make whitespace (which was what i used as well in this PR to fix the whitespaces)
there might be a way also with make fmt need to check
There was a problem hiding this comment.
I recall that i did try make fmt (and also retried now),
it doesn't fix it, at least not out of the box
will fix the text string
|
Rebase |
| @@ -9,7 +9,7 @@ function fix() { | |||
| function check() { | |||
| invalid_files=$(git ls-files -- ':!vendor/' | xargs egrep -Hn " +$" || true) | |||
| if [[ $invalid_files ]]; then | |||
| echo 'Found trailing whitespaces. Please remove trailing whitespaces using `make fmt`:' | |||
| echo 'Found trailing whitespaces. Please remove trailing whitespaces using `make whitespace`:' | |||
There was a problem hiding this comment.
Please move this fix to the previous commit.
Signed-off-by: Or Shoval <oshoval@redhat.com>
Signed-off-by: Or Shoval <oshoval@redhat.com>
|
Addressed comments |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AlonaKaplan 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 |
kubevirt-bot
added
the
approved
|
/retest |
Also fix the needed whitespace (using
make whitespace)