Feature: Require "Approve" from person who set "Needs work"

[artem-zinnatullin]

If someone has set "Needs work" on a PR, would be great to require "Approve" from this person before merge because currently Bitbucket resets "Needs work" on each push to a PR which can lead to merge without required fix (especially with automerge).

[artem-zinnatullin]

One of the suggestions for implementation — lookup PR Activity and block merge if one of the approvers has set "Needs work" but no "Approve" from this person has happened after it.

[monitorjbl]

Fixed by #57

[artem-zinnatullin]

Wait wait wait! #57 solves another issue #56

[artem-zinnatullin]

Lemme try to describe what this feature request is about:

Imagine you have set 3 required reviewers but your team is let's say 6 developers. Then imagine one of them marked PR as Needs work but other 3 marked PR as Approved.

Then author of the PR pushes some changes to PR, BitBucket automatically removes all Needs work statuses.

In current version of Plugin even with #57 PR will be automerged because 3 required approves were gained.

What we would like to do is to block merge until person who did not Approve the PR after he/she marked PR as Needs work.

TL;TR: if after Needs work reviewer did not Approve the PR, merge should be blocked.

Hope that makes sense? :)

[monitorjbl]

Hm, so the current code doesn't do that? I didn't test the automerge block before I released this, but I did test the PR veto and it worked. I'm kind of surprised that this line doesn't effectively call this method through the PullRequestService.

[artem-zinnatullin]

Well, the problem is that any push to the PR branch removes all Needs work marks and then automerge happily happens :)

[monitorjbl]

Interesting...does it not also remove the Approved? We still use Bitbucket 4.3 where I work, so I haven't gotten to mess around with this feature outside of the SDK.

[artem-zinnatullin]

Nono, we're on 4.5.2 and it removes only Needs work on every push in our repos and I wasn't able to find any setting for that.

[monitorjbl]

Well, that's something of a problem...can we continue this discussion on #56 ?

[artem-zinnatullin]

ok!