-
Notifications
You must be signed in to change notification settings - Fork 3k
Revert "Switch to golangci-lint" #2865
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
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: baude The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: baude <[email protected]>
|
if this works, merge it and hope we're back to just working ... |
|
LGTM but please explain why it’s needed to revert |
|
I am attempting to make golangci-lint work in #2867 |
because it broke us on prow and our CI is dead |
|
Why did we merge this when it wasn't passing CI?
…On Sat, Apr 6, 2019, 10:54 Brent Baude ***@***.***> wrote:
LGTM but please explain why it’s needed to revert
because it broke us on prow and our CI is dead
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#2865 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHYHCPOMNQcL6SRqA_n_IheB8jpsLh8Uks5veLUQgaJpZM4cf7jE>
.
|
|
Regardless, I'm with Brent here. If we don't have a fix soon I'm going to
merge the revert so we can unblock PRs
…On Sat, Apr 6, 2019, 10:58 Matthew Heon ***@***.***> wrote:
Why did we merge this when it wasn't passing CI?
On Sat, Apr 6, 2019, 10:54 Brent Baude ***@***.***> wrote:
> LGTM but please explain why it’s needed to revert
>
> because it broke us on prow and our CI is dead
>
> —
> You are receiving this because your review was requested.
> Reply to this email directly, view it on GitHub
> <#2865 (comment)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AHYHCPOMNQcL6SRqA_n_IheB8jpsLh8Uks5veLUQgaJpZM4cf7jE>
> .
>
|
|
because chris asked dan to ... i alerted chris to the problem immediately but he went on to PTO |
|
the core issue is the linter has to be setup for the tests we do in openshift containers too. that was not done and i think we should revert until chris is back from pto. |
|
@baude Based on Steve's comments elsewhere - do we want to update the Dockerfiles for ci/prow/lint instead of this? It seems like that's our last blocker on this. I'm still open to merging this is we need to unblock stuff ASAP. |
|
let's merge. It is blocking all our PRs. We can reintroduce golangci-lint when it works. /lgtm |
Signed-off-by: baude [email protected]