Support multiple PR reviews

[byzyk]

Is your feature request related to a problem? Please describe.

There are a lot of people on our team so we require at least 2 PR approvals on some branches. Unfortunately, the bot doesn't seem to recognize this. It adds APPROVED label as soon as at least one approval is received, which is confusing.

Describe the solution you'd like

It'd be great if the bot could access repo settings and check how many approvals are required before updating labels.

Describe alternatives you've considered

N/A

Additional context

I'd be more than happy to help with the PR.

[gundalow]

When you have a large backlog, I wonder if a label indicating one more review is needed would help keep the backlog moving.

new_pre --> PR: unreviewed --> PR: reviewed-changes-requests --> PR: partially-approved --> PR: reviewed-approved --> PR: merged

(may need a better name)

Then maintainers can search for PRs that need just a little more help getting merged.

[sotayamashita]

@gundalow I like your idea.

PRTriage would triage pull request with the following rule:

• It adds PR: unreviewed when a pull request is created.
• It adds PR: reviewed-changes-requests when one of the reviewers ask to need a change
• It adds PR: partially-approved when one of the reviewers approves the change.
• It adds PR: reviewed-approved when every reviewer approves the change.
• It adds PR: merged when that pull request is merged.

[sotayamashita]

Incidentally, I would like to change the name of PR: reviewed-approved because I think this name is not intuitive. PR: ready-to-merge would be more intuitive what do you think?

[gundalow]

Incidentally, I would like to change the name of PR: reviewed-approved because I think this name is not intuitive. PR: ready-to-merge would be more intuitive what do you think?

or maybe just PR: reviewed?

[gundalow]

How would the bot know how many reviews are needed? Could it read branch protections via the API? If that isn't set, default to 1, which matches the current functionality.

[sotayamashita]

@gundalow

or maybe just PR: reviewed?

It is very simple but I think we cannot guess the next action of PR and PR: reviewed might be recognized as approved or changes-requests.

[sotayamashita]

How would the bot know how many reviews are needed?

We can get the number with https://developer.github.com/v3/pulls/review_requests/#list-review-requests

Could it read branch protections via the API?

We can get the number with https://developer.github.com/v3/repos/branches/#get-branch-protection

If that isn't set, default to 1, which matches the current functionality.

I got it.

[byzyk]

Just a small comment from my end if I may.

I'm not sure whether PR: ready-to-merge should be used since PR might receive all required approvals but CI or other GitHub checks might fail, meaning despite all the approvals PR is not really ready to merge.

[maestromac]

Perhaps keeping it reviewed-approved is the path of least resistance for now and we should leave it for #88

[gundalow]

Perhaps keeping it reviewed-approved is the path of least resistance for now and we should leave it for #88

Sounds good to me.

[allout58]

I'm planning to take a crack at a PR for this issue this afternoon. Any other thoughts before I start working on it?

[allout58]

It looks like https://developer.github.com/v3/repos/branches/#get-branch-protection doesn't actually return the required_approving_review_count property like the documentation claims. This is probably a bug with the API, but it means I can't really verify that my changes on that front do anything.