Skip to content

meta: add policy to land PRs without approval #336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mmarchini wants to merge 1 commit into from
Closed

meta: add policy to land PRs without approval #336

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions COLLABORATOR_GUIDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,6 @@ with the following differences:
- There's no minimum wait time for landing a pull request. Pull requests can
land as soon as they get approved by at least one Collaborator.
- Approval must be from Collaborators who are not authors of the change.
- If the pull request was open for three days (72 hours) without reviews, and
Copy link
Contributor

@cjihrig cjihrig Feb 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not to intentionally slow llnode down any further, but what about 7 days to match the one approval clause from https://github.com/nodejs/node/blob/master/COLLABORATOR_GUIDE.md#code-reviews.

Copy link
Contributor Author

@mmarchini mmarchini Feb 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely opposed to 7 days, but I don't think "to match the one approval clause from core" is a good reason to do so. I'm open to hearing other arguments to use 7 days instead of 3 though.

On a counter-argument to 7 days, if there is a bug and a PR with a fix doesn't get a review, it will now take seven days to release a fix. We could have a shorter waiting period for bug fixes, but that seems to overly complicate the policy.

Copy link
Contributor

@cjihrig cjihrig Feb 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To counter your counter-argument, if there is a bug under this proposal, it will take 3 days to release a fix 😄. I guess any amount of waiting will be an arbitrary amount. I only recommended going with 7 days because that was the agreed upon answer to this exact same problem in core. At the end of the day, you're the one doing literally all of the work, and I'm not going to block this PR.

it was opened by a llnode Collaborator, the pull request can land without
approvals.