Repository navigation
Require keeping an unreviewed PR open for a period of time #541
Description
In order to encourage reviews, and to prevent PRs from being created and merged without reviews within a short period of time (sometimes within one hour!), it has multiple times been suggested (on Discord, informally in discussions on PyCon) to add tooling that prevents an unreviewed reviewed PR to be merged unless a predetermined period of time has passed.
I think we should add a check that for unreviewed PRs will require a certain period of time to pass before the PR can be merged. This check will reset if new commits are added. If a review is performed, the check is dismissed. If a review if performed and new commits are added, the check is reset and a new review must be performed in order to dismiss the check again.
For discussion:
- Should this check be dismissed only when a core dev or triager has reviewed the PR, or should we allow external reviews?
- How long should the delay be? I suggest somewhere between 1 and 2 weeks. IMO, 2 days or even 3 days is too short.
Activity
hauntsaninja commentedon Jun 10, 2024
Maybe you could pull statistics from recent PRs so we know what the impact would be?
I'm worried this will force roundtrips that aren't particularly valuable
erlend-aasland commentedon Jun 10, 2024
[additional context added by me]
It might do; it may also be valuable in some situations. If it turns out to be more of a nuisance, we can easily disable that functionality.
AlexWaygood commentedon Jun 10, 2024
I like this idea overall, but I think it would be good to exclude docs-only PRs from this check.
I don't like this much, as I think it would disincentivise people from merging in
mainto the PR branch just before merging the PR. That's usually a good idea, as it helps prevent merge races where a outdated CI run is green on the PR but the tests end up failing onmainerlend-aasland commentedon Jun 10, 2024
That makes sense.
For external PRs, this is not a problem; the person merging in
mainwill certainly be a core dev, and thus be in the position to immediately also approve the PR. For PRs created by core devs, it would of course be more irritating. I guess you are right it will quickly become a pain point.sobolevn commentedon Jun 10, 2024
We can use a comment with
/recheck-review-statuscommand or similar to use together withAA-Turner commentedon Aug 12, 2025
Closing, stale.
erlend-aasland commentedon Sep 18, 2025
I still think we should consider this.