Define a policy for acceptance before merge

[rengolin]

We currently need approvals to merge, either official flags in Phab or the text "LGTM" on some comment. GH allows us to go one step further and define what kind of approval we get before allowing someone to press the "merge" button.

We also have a less defined policy to when multiple approvals are necessary, or how long to wait for new comments before merging after the first approval.

Given that our developers all have GH accounts and clicking on "accept" is easier than writing "LGTM", I think it's trivial to define that someone must approve the commit before merging (automation-wise), but there are questions as to who or how many need to approve before merge.

Things to consider:

• Can someone approve their own commits? What if someone says "LGTM with nits" and the nits are resolved but the reviewer never replies / presses the button?
• Can we set different automation rules for different types of reviews? Examples:
  • Must approve all patches in a stacked PR to merge each
  • Must wait at least N days from posting / M days from first approval to allow the merge button
  • Must have at least K approvals before merge on core/docs/policy changes

[llvmbot]

@llvm/issue-subscribers-infrastructure

[hubert-reinterpretcast]

How does this interact direct pushes to main and reverts/reapplies?

[rengolin]

How does this interact direct pushes to main and reverts/reapplies?

It doesn't. This is just an extension of the policy for reviews.

I don't want this to change the current policy on direct access (how, why, etc).

[tstellar]

Do we need a new policy for pull requests? Can we just use the same policies we have for Phabricator?

[crtrott]

I think the mechanical way to do this in github is to have a code owner file, and require code owner approval, and also only allow code owners to click merge. This may however be connected to the access rights on the main branch. I.e. to prohibit folks from merging, you also need to prohibit them from directly pushing?