-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Fix misleading message in "make ci" when imports formatted incorrectly #8045
Conversation
Hi @johngmyers. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test |
e323c4e
to
a23be7b
Compare
@johngmyers thank you for bailing me out :) i went back to the drawing board on this. I'm wondering if targets |
I don't see a |
Sorry, you're correct. I was comparing the targets They seem to do essentially the same right now. I thought the intention of the travis target was to be like a I would argue that we should remove one or the other (probably by aliasing, so we don't break folks). |
|
Are there any issues with this PR as currently proposed or can it go in? I have a conflicting change I would like to propose as a new PR. |
/lgtm sure. we can come back to this one. thanks @johngmyers |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: geojaz, johngmyers 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 |
/retest |
/kind bug
If the current commit contains a file with an import formatting error, such that make verify-goimports would fail, then a make ci will give the wrong error message, giving the user the wrong instructions for fixing the problem.
The added goimports will "fix up" the problem, causing the later verify-goimports to incorrectly pass. Then, verify-gomod will notice the dirty workspace and incorrectly tell the user to run
go mod tidy
.This bug was introduced by #8023
The commit introducing the bug was reviewed and approved by @geojaz, so
/assign @geojaz