Repository navigation
meta: number of approval ambiguity #21565
Description
It seems we have some ambiguity in docs.
https://github.com/nodejs/node/blob/master/COLLABORATOR_GUIDE.md#code-reviews:
All pull requests must be reviewed and accepted by a Collaborator with sufficient expertise who is able to take full responsibility for the change. In the case of pull requests proposed by an existing Collaborator, an additional Collaborator is required for sign-off.
https://github.com/nodejs/node/blob/master/doc/guides/contributing/pull-requests.md#step-10-landing
In order to land, a Pull Request needs to be reviewed and approved by at least one Node.js Collaborator and pass a CI (Continuous Integration) test run.
Evidence of confusing:
So how many approval do we need for a PR when the author is a Collaborator?
Activity
bnoordhuis commentedon Jun 27, 2018
Just one. That section aims to convey that collaborators can't self-approve.
mmarchini commentedon Jun 28, 2018
We only require two approvals when the PR is being fast-tracked.
vsemozhetbyt commentedon Jun 28, 2018
There was also a proposal to require two approvals if the PR was initiated by a Collaborator (in an issue etc.) but created by a non-Collaborator so that a Collaborator could not self-approve indirectly. But I cannot remember if there was a consensus about this change.
gabrielschulhof commentedon Jun 28, 2018
(scratches head) so, may I land #21318? 🙂
Trott commentedon Jun 28, 2018
Only one approval is required, although as a matter of practice starting a few months ago, I personally always try to get a second review and have never failed as far as I can recall.