Thinking Point: Will requiring 2 Collaborators' POV improve reviews?

[refack]

I can't believe this is coming from me, but I was thinking we should consider increasing the review requirements by a little bit. From recent personal experience I find that reviews are better when there are at least two participants voicing opinions. So I suggest we require that for a PR to be land 2 Collaborators need to participate in the review. Either the the PR will need 2 approvals, or if the OP is a Collaborator, then OP + 1. Although the more experienced non-Collaborator contributors can rationalize their decision processes very well, some of the newer contributors yield to reviews too quickly, IMHO leaving the review process a bit one sided. _{Personally I feel that I do a better job when challenged. A little bit like in Socratic dialogue or 2000's style Pair programming}

Something to think about?

[Trott]

@refack Can you point to a concrete example where additional reviews would have been useful? It would help me to have a real-world example I could consider.

[mcollina]

I'm not exactly in favor of this, as most of the regression that I was involved in were signed off by multiple collaborators, so the bug was not easily spottable with a code review.

[targos]

I also think we need concrete examples to make this kind of decisions.

[refack]

First of all I'm not talking about regressions, I'm talking about optimizing quality.

I didn't want to ref PRs because I didn't want it to be construed as criticism of contributors, so if anybody that's reading this find their name referenced, this is not criticism! IMHO contributions from users are the lifeblood of this project, and personally I consider them priceless (big, small, accepted, or rejected). As I see it, the Collaborators are here mostly to help things along...

I have a case from just now: https://github.com/nodejs/node/pull/13723 - this could have landed, but a fresh pair of eyes brought a new idea https://github.com/nodejs/node/pull/13723#discussion_r122772600. (also before that for some reason I decided to review again and found https://github.com/nodejs/node/pull/13723#discussion_r122612829, which IMHO is worth discussing, and https://github.com/nodejs/node/pull/13723#discussion_r122691069)

[addaleax]

@refack I think what your example shows is that it’s a good idea to apply 48/72-hour for non-trivial PRs, not necessarily anything relating to the # of reviewers.

[refack]

@refack I think what your example shows is that it’s a good idea to apply 48/72-hour for non-trivial PRs, not necessarily anything relating to the # of reviewers.

I was just thinking about that too, extend the cooking time

[Trott]

I'm going to close this, but feel free to open another issue in a relevant active repository (TSC perhaps?) and include a link back to this issue if this is a subject that should receive continued attention.