Possible to add commits after PR approval without owner review

[dippynark]

Maybe this is expected, but this PR was LGMT'd and approved, but the tests weren't passing so I fixed them. The LGTM label was removed as expected, but the bot still merged the PR since the approved label remained, so I could have sneaked in malicious code.

Assuming Lighthouse is being used as the webhook handler, I think there are two solutions to this:

1. Change the tide/keeper plugin to exactly match the GitHub approve status (that is, if there are still checks that haven't completed like Lighthouse Merge Status, then do not attempt to merge). This way, GitHub's built-in merge protection can be enabled (e.g. requiring 1 approver, or requiring particular checks passing) without clashes. For example, in my personal testing, I see things like this because of this misalignment: Update README.md dippynark/kfmt#19 (comment)
2. Disable the approve plugin and only use the lgtm plugin to gate merging (since new commits cause the lgtm label to be removed) and change the Lighthouse tide/keeper config to be something like:

queries:
- labels:
   - lgtm
   missingLabels:
   - do-not-merge
   - do-not-merge/hold
   - do-not-merge/work-in-progress
   - needs-ok-to-test
   - needs-rebase
   repos:
   - jenkins-x/lighthouse

[ankitm123]

Yeah, it only removes the lgtm label, but not the approved label. I think a simple fix would be to just remove the approved label once we see a new commit.
The problem with using only lgtm is that for owners lgtm will act as approve, and add approve label as well.
May be that wont be a problem if we disable the approved plugin ...

Change the tide/keeper plugin to exactly match the GitHub approve status (that is, if there are still checks that haven't completed like Lighthouse Merge Status, then do not attempt to merge). This way, GitHub's built-in merge protection can be enabled (e.g. requiring 1 approver, or requiring particular checks passing) without clashes

What about other scm providers? I have not tried lighthouse with gitlab or bitbucket, will this work with them?

[dippynark]

Yeah was thinking of disabling the approve plugin (at least that's what I've done for my personal projects), but the approve plugin also has lots of great benefits so it's a shame to not have them. Getting the approve plugin to remove the approve label seems like a good shout (I guess all the code can be copied from the lgtm plugin).

I have not tried with those other providers either. If the approve plugin also just approved the PR in the SCM provider sense then that might help (again, not too sure what that'd mean for the other providers), but that might not be as much work.

[ankitm123]

/assign

Once I have the PR up, u can review it.
Started the work on it.