Test-infra: RFC: Incorporate LGTM After Commit Munger into Prow LGTM plugin

[nlandolfi]

I propose to add the functionality implemented in this munger to this prow plugin.

Background

When PR commenter/reviewer adds a comment with

/lgtm

the Prow LGTM plugin will add the "lgtm" label to the pull request.

Motivation

Suppose, after a reviewer has commented /lgtm, the author updates the pull request. We want the "lgtm" label to be removed from the PR. This behavior is currently implemented via a Munger.

Proposal

1. In the prow plugin lgtm, add a plugins.PullRequestHandler
2. On "synchronize" actions, remove lgtm label, if it exists, from the PR.

[spxtr]

Sounds good to me.

[kargakis]

/area prow
/area mungegithub

[kargakis]

/assign nlandolfi

[BenTheElder]

@cjwagner @xiangpengzhao maybe we should pick this one up now? this seems like a small thing to (re)implement.

[nlandolfi]

@BenTheElder sgtm

[MorrisLaw]

I'll work on this issue, @BenTheElder @nlandolfi @kargakis @cjwagner @xiangpengzhao @spxtr

[xiangpengzhao]

@MorrisLaw cool, thanks!

[MorrisLaw]

Hey, when I run the test-infra/prow/plugins/lgtm tests. Specifically the tests in the function TestLGTMComment. It gives me the following error:

# test-infra/prow/plugins/lgtm
./lgtm.go:63:35: cannot use pc.Logger (type *"k8s.io/test-infra/vendor/github.com/sirupsen/logrus".Entry) as type *"test-infra/vendor/github.com/sirupsen/logrus".Entry in argument to handle
FAIL    test-infra/prow/plugins/lgtm [build failed]
Error: Tests failed.

This is the code that references handle in test-infra/prow/plugins/lgtm.go:

func handleGenericComment(pc plugins.PluginClient, e github.GenericCommentEvent) error {
    return handle(pc.GitHubClient, pc.Logger, &e)
}

And here is the signature for the handle function itself.

func handle(gc githubClient, log *logrus.Entry, e *github.GenericCommentEvent) error { ... }

Is anyone else getting this error?
I have some changes that I made, but they introduce a similar error. But if it's easier to debug, I can make a PR for code review/suggestions.

Thank you!

Figured out my problem, no longer an issue.

[MorrisLaw]

Ok, so apparently I was in the wrong directory. I was in ~/go/src/test-infra as opposed to ~/go/src/k8s.io/test-infra. Thank you @cjwagner. The tests on my end, now work as expected.

[cblecker]

Where are we at with this one? This is a critical piece to retiring the mungers. Right now if a repo doesn't have mungegithub on it, then a PR can be changed from the time of LGTM and tide will still merge.

From what I can tell:

• @nlandolfi had a WIP PR, but isn't able to carry it forward
• @MorrisLaw said he would work on it, but I don't see a linked PR

cc: @cjwagner @BenTheElder @spxtr @fejta @spiffxp

[MorrisLaw]

Hey @cblecker , I took a break for awhile but I'm going to start working on this again. Feel free to make the PR if you have it. But my goal is to get a PR in, within the next few days. Unless of course someone else makes the PR.

[cblecker]

@MorrisLaw Thanks for checking in! Yeah, please go for it if you have the cycles (I won't in the next couple days). If you run into issues or need help, please let us know!