pr-harmony
pr-harmony copied to clipboard
monitorjbl
Any way to block automerge if there is "Needs work" from one of the reviewers?
Imagine we have 6 developers in the team and 4 reviewers are required to merge (auto or manually) the PR.
If one of the reviewers sets "Needs work" on a PR, automerge still happens if 4 reviewers will approve it and CI will pass.
Is there any way to prevent auto merge if there is at least one "Needs work" on a PR? Thanks!
Almost finished PR with "Block merge if PR needs work" option, will open soon.
The easiest path is to track when a required reviewer flags a PR as needing work. Bitbucket doesn't make it easy or clean to have per-item persistence, so I'm somewhat hesitant to start introducing that unless it's absolutely necessary.
Does the clearing of the "Needs work" value happen in the current version of Bitbucket? It's possible this is behavior they fixed later on.
Bitbucket doesn't make it easy or clean to have per-item persistence, so I'm somewhat hesitant to start introducing that unless it's absolutely necessary.
We can simply lookup PR Activity list and check if reviewer who set Needs work then set Approved or not.
Does the clearing of the "Needs work" value happen in the current version of Bitbucket? It's possible this is behavior they fixed later on.
We're definitely seeing this on 4.5.2, but it might be caused by our workflow because we always squash commits locally on each change in a PR and then git push --force it to the PR branch.
Are you referring to this?
https://developer.atlassian.com/static/javadoc/stash.old-perms-pre-feb4/1.3.0/api/reference/com/atlassian/stash/pull/PullRequestService.html#getActivities(int, long, com.atlassian.stash.util.PageRequest)
I haven't used that particular API method before, does it expose the full history of approvals for each participant?
Yes, I'm referring to this API, I haven't used it yet, but looks like we should be able to get as much entries as we need.
Moreover we can optimize it: knowing list of reviewers we can stop looking through PR Activity if we found that latest statuses of the reviewers are not blocking it from merge. For example if PR has 3 reviewers and first page of PR Activity shown that all of them Approved/etc PR we don't need to lookup through other pages of PR Activity.
If it has the data, then I think this is approach to take. Did you want to take a crack at this?
I can, but I will have time for it only in 1-2 weeks. I can review your PR if you'll take this :D
Great! Thank you for doing this and for plugin in general, really helps us 👍
Sorry for the delay, the holidays kinda wrecked my free time for a while. I tried to test this out on 4.5.2 and I'm not actually seeing the behavior you're describing. Here's what I did:
- Configured the PR Harmony of the repo to have: a. 2 default reviewers (user1 and user2) b. 2 required reviewers (user1 and user2) c. 1 required reviews d. Block on needs work e. Automerge on master
- Configure the Pull Request section of the repo to require at least one successful build
- Opened a PR as user3
- Had user2 approve the PR
- Had user3 flag the PR as needs work
- Sent a build success notification to the REST API
No prior approvals were removed and the PR stayed open. I confirmed that the automerge did work when the PR was fully approved. Is there another plugin or setting that's causing this issue for you?