Node: meta: number of approval ambiguity

[vsemozhetbyt]

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:

https://github.com/nodejs/node/pull/21318#issuecomment-400483863

So how many approval do we need for a PR when the author is a Collaborator?

[bnoordhuis]

Just one. That section aims to convey that collaborators can't self-approve.

[mmarchini]

We only require two approvals when the PR is being fast-tracked.

[vsemozhetbyt]

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]

(scratches head) so, may I land https://github.com/nodejs/node/pull/21318? 🙂

[Trott]

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.