Only update branch once approval was given

[NotSoImportant]

Please add a configurable option to only update a PR branch once reviewers have approved (if approval is required on this branch)

This would help to reduce the number of useless builds triggered by updating the branch frequently (see
#145 (comment) last section).

Readme suggests bulldozer to be useful if you "Have a lot of active development", but actually here it is making the build machines running hot.

Another good way to taggle this problem would be to implement #128

[wrslatz]

I'm considering contributing this feature.

I'm trying to figure out how to use the GitHub API via go-github to check if a PR has met review requirements and don't see exactly what I expect we would need. I don't think checking mergeable state of the PR would give us what we need.

I also don't see anything under the Pull Reviews API that summarizes the state of approvals across the PR. We could check that the PR has approvals and none are REQUEST_CHANGES state via list reviews, but I don't think checking this would be sufficient to ensure that the PR has met all approval requirements for merge.

[wrslatz]

This Stack Overflow post seems related but doesn't seem to offer a complete solution for this use case.

[bluekeyes]

Bulldozer should have permission to read branch protection details. The way I'd initially approach this is using the get pull request review protection API to get the approval conditions, then use the endpoint to list reviews to see if there are approving reviews that satisfy the conditions.

Unfortunately, I don't think this provides a way to test for required code owner reviews, unless there's an API to get the code owners for the files changed in the PR. We might have to accept that this feature would not fully implement the same conditions as GitHub.

[wrslatz]

Do we think PullRequestReviewDecision from the reviewDecision on the Pull Request in the GraphQL API would give the summary info on approvals that we need?

I'm also happy to make a discussion / post on the appropriate GitHub feedback repo to get some additional ideas on this one.

[wrslatz]

Another possibility, from this Stack Overflow answer, would be to simply check if the PR merge state is BLOCKED. I'm not sure how this value is calculated when a PR is both out of date and blocked though...

If this field takes into account both required reviews and statuses (need to confirm), it might be possible to implement both this feature request and #259 in a single check and config option? Edit: we would still need custom logic if we wanted to check specific statuses are passing vs the ones required in branch protections.

[bluekeyes]

The mergeStateStatus field in GraphQL sounds most promising. GitHub obviously has all of this information available to render the appropriate merge button state, so hopefully this field is effectively the same information that is currently used by the UI. If it also returns BLOCKED when required status checks are not passing, that could simplify things further by removing the need to look that information up in branch protection.

There are also some other cases where Bulldozer optimistically attempts a merge and handles any error. Having accurate blocking information could avoid this too.

One complication might be the difference between the BEHIND and BLOCKED values. If Bulldozer is performing an update, the PR should be behind, but it's unclear which of these two GitHub will use when a PR is both behind and has approval or other conditions that are not met. Ideally, BEHIND is only used when all conditions except for the up-to-dateness are satisfied.

This would be the first use of the GraphQL API in Bulldozer, but I don't think that's a problem. The main change for users is that they must also specify the v4_api_url in the configuration if they are not already.

[wrslatz]

Looks like that field is also in beta and subject to change, so not sure the appetite to use that field before that part of the API stabilized.

Preview notice

mergeStateStatus is available under the Merge info preview. During the preview period, the API may change without notice.