-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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.
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
meta: reduce 7 day rule to 4 days (96 hours) #34735
Conversation
There is no evidence that requiring PRs that only have a single review will receive more review after a week. Let's make things easier and reduce it down to 96 hours (double the minimal review time).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still prefer #33879, but this is definitely an improvement :)
Also worth noting: we now have codeowners in place (although we need to add more to the list), which increases the chance of folks interested in a PR to receive and see the notification (since it becomes a priority notification) for new PRs. |
So do I ;-) |
@jasnell It only needs your approval ;) |
Heh .. well, that was just because I wanted to see the codeowners stuff move forward! Now that it has, more then happy for my review to be dismissed. I just wasn't sure if you wanted to keep pushing it forward |
FWIW if the "daily landing report" is a requirement to drop minimum wait time, I'm more than happy to send a PR for it this week :) |
@addaleax... if you'd like to reopen your original PR, I will clear my objection on that one. |
Closing in favor of #33879 |
There is no evidence the 7 day rule encourages additional review. Let's make things easier and reduce it down to 96 hours (double the minimal review time).
/cc @addaleax @nodejs/tsc
Checklist