[Rust-VMM] Project gate keeping

[Florescu, Andreea]

I prefer having two approvals either from code owners or gatekeepers when there are no code owners.
Merging a PR with only one review should be an exception from my point of view. More often than not 2 different people looking at the code will find different bugs or other issues (lack of tests, documentation, others?).

GitHub allows administrators to merge PRs even though there are not 2 reviews, so we can merge PRs with one review if needed, but I think we should have 2 reviewers as the rule.

Andreea
________________________________________
From: Paolo Bonzini <pbonzini at redhat.com>
Sent: Friday, June 7, 2019 6:28 PM
To: Samuel Ortiz; rust-vmm ML
Subject: Re: [Rust-VMM] Project gate keeping

On 07/06/19 10:23, Samuel Ortiz wrote:
> - Have a team of 8-10 rust-vmm gatekeepers. 2 randomly selected
>   gatekeepers will be automatically assigned to any incoming PR.

Would gatekeepers be automatically given access to all rustvmm repos?
Is that possible from the GitHub UI or do you have to do that manually?

> - Any gate keeper is free to assign someone else from the gate keeper
>   team if she/he does not have the badwidth/knowledge to review the PR.
> - Encourage CODEOWNERS[1] file additions. Not all repos may need such
>   ownership so I don't think this should be mandatory.
> - Any PR will be mergeable when any of the below conditions are met:
>   * When a repo has a CODEOWNERS file, 1 code owner and 1 gate keeper
>     approved it.
>   * On CODEOWNERS-less repos, 2 gate keepers approved it.

I think even that is too much; I don't see a reason to overload
gatekeepers with the task of closing PRs.  My proposal is:

- up to 2 randomly selected code owners, or gatekeepers if there are <2
code owners will be automatically assigned to any incoming PR

- as far as the GitHub UI is concerned, just one approving review from a
code owner or gatekeeper is enough to merge

- code owners and gatekeepers however are warmly encouraged to wait for
a review from a second person for anything except obvious bugfixes, or
to wait for a second code owner or gatekeeper to actually do the merge
and close the PR.

> - Initial PRs for empty crates could be handled differently and
>   informally require more approvals from different people.

Agreed.

Paolo

_______________________________________________
Rust-vmm mailing list
Rust-vmm at lists.opendev.org
http://lists.opendev.org/cgi-bin/mailman/listinfo/rust-vmm



Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.