Skip to content

Commit

Permalink
doc: leave pull requests open for 72 hours
Browse files Browse the repository at this point in the history
Currently, we have a 48/72 rule for how many hours a pull request should
be left open at a minimum. Unfortunately, whether a pull request should
be left open for 48 or 72 hours is often unclear. The 72 hours is
required if it is a weekend. If I open a pull request on a Friday
morning, does it need to stay open 48 hours or 72 or something in
between? Does it matter if I'm in one time zone or another?

The 48/72 rule predates our fast-tracking process. Given the ability to
fast-track trivial pull requests, there should be little disadvantage to
leaving significant changes open for 72 hours instead of 48 hours, and
arguably considerable advantage in terms of allowing people sufficient
time to review things.

So to simplify, standardize on 72 hours. Weekend or not, 72 hours. Easy.

PR-URL: #22275
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Coe <bencoe@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
  • Loading branch information
Trott committed Oct 4, 2018
1 parent 26af728 commit 2f117e1
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 7 deletions.
8 changes: 4 additions & 4 deletions COLLABORATOR_GUIDE.md
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -171,10 +171,10 @@ agenda.
### Waiting for Approvals ### Waiting for Approvals


Before landing pull requests, sufficient time should be left for input Before landing pull requests, sufficient time should be left for input
from other Collaborators. In general, leave at least 48 hours during the from other Collaborators. In general, leave at least 72 hours to account for
week and 72 hours over weekends to account for international time international time differences and work schedules. However, certain types of
differences and work schedules. However, certain types of pull requests pull requests can be fast-tracked and may be landed after a shorter delay. For
can be fast-tracked and may be landed after a shorter delay. For example: example:


* Focused changes that affect only documentation and/or the test suite: * Focused changes that affect only documentation and/or the test suite:
* `code-and-learn` tasks typically fall into this category. * `code-and-learn` tasks typically fall into this category.
Expand Down
5 changes: 2 additions & 3 deletions doc/onboarding.md
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -138,8 +138,7 @@ onboarding session.
* There is a minimum waiting time which we try to respect for non-trivial * There is a minimum waiting time which we try to respect for non-trivial
changes so that people who may have important input in such a distributed changes so that people who may have important input in such a distributed
project are able to respond. project are able to respond.
* For non-trivial changes, leave the pull request open for at least 48 hours * For non-trivial changes, leave the pull request open for at least 72 hours.
(72 hours on a weekend).
* If a pull request is abandoned, check if they'd mind if you took it over * If a pull request is abandoned, check if they'd mind if you took it over
(especially if it just has nits left). (especially if it just has nits left).
* Approving a change * Approving a change
Expand Down Expand Up @@ -215,7 +214,7 @@ needs to be pointed out separately during the onboarding.
* Run CI on the PR. Because the PR does not affect any code, use the * Run CI on the PR. Because the PR does not affect any code, use the
`node-test-pull-request-lite-pipeline` CI task. `node-test-pull-request-lite-pipeline` CI task.
* After one or two approvals, land the PR (PRs of this type do not need to wait * After one or two approvals, land the PR (PRs of this type do not need to wait
for 48/72 hours to land). for 72 hours to land).
* Be sure to add the `PR-URL: <full-pr-url>` and appropriate `Reviewed-By:` * Be sure to add the `PR-URL: <full-pr-url>` and appropriate `Reviewed-By:`
metadata. metadata.
* [`node-core-utils`][] automates the generation of metadata and the landing * [`node-core-utils`][] automates the generation of metadata and the landing
Expand Down

0 comments on commit 2f117e1

Please sign in to comment.