In response to Nathan's message on the previous thread: https://www.mail-archive.com/[email protected]/msg13877.html
> I would like to point out that if we have to wait 72 hours, then hold a > vote lasting 72 more hours, that's 6 days total. Not necessarily the end of > the world but could be significant if there's other work in the pipeline > that depends on the stuck PR. Honestly, I think that's fine. If a reviewer has left a change request it was likely for good reason. I think 72 hours to respond to questions about their request/new changes is a reasonable time frame, although I'd be willing to hear other perspectives. I think a 6 day block on a PR with requested changes is reasonable, there shouldn't be a rush to merge PRs faster than that if there isn't a consensus of approvals. Critical PRs can still be merged in less than 6 days provided a reviewer doesn't find issue with them. That being said, I think votes are the ultimate ruler on Apache projects, so really #2 is more of a guideline. If someone _really_ thinks the PR needs to be merged ASAP, they can run a vote immediately to request that an exception to #2 be made and the review dismissed immediately. I don't know why that would be necessary, but it would only cause a 3-day delay. I believe we still have the rule of no self-merging commits, even if you received enough approvals. This new proposal is in the same spirit of community approval & concurrence. If someone who was trusted enough by the project to be a committer has left a binding "request changes" review, that feedback should be respected and not dismissed until the reviewer has agreed. Best, Matteo On Tue, Nov 11, 2025 at 1:11 PM Matteo Golin <[email protected]> wrote: > Hello everyone, > > I am proposing a vote for a new rule surrounding PR reviews for the NuttX > project. I have started this discussion thread to review the proposal > first. > > Below is my original message: > > As you might know, it is possible to "request changes" on GitHub when > reviewing > a PR. If changes are requested, it is not possible for the PR to be merged > until > the reviewer who has requested changes dismisses their request or approves > the > PR. > > However, an unfortunate ability on GitHub is that anyone with write access > (or > higher) privileges on the repository can dismiss a reviewer's change > request. > Unfortunately, this has been used at least twice recently to merge a PR for > which reviewers requested changes, which is quite frustrating for the > reviewer(s) and does not respect their feedback. I don't think this > ability fits > with NuttX's ideals of ensuring quality code, and I also think it > alienates the > reviewers (we have very few of them, so that is bad). > > My proposal for the rule is as follows: > > 1. A change-request made by a reviewer can only be "self-dismissed". This > indicates the reviewer has been satisfied by the changes made or they have > been > convinced that their change request is not necessary. > > 2. If the reviewer is not responsive due to absence (minimum 72 hours), or > other > reviewers of the PR believe that the reviewer's concern is invalid, a VOTE > can > be made on the mailing list to overturn the change request. If the VOTE > passes, > this is the only scenario where the request can be dismissed by someone > else. > > I have included item 2 as a contingency, although I don't expect this > scenario > to happen often if at all. I suggest that if the vote passes, this rule be > included in the contributing guidelines (if there are better locations, > please > suggest them). > > I would also just take this opportunity to say: if you have comments > regarding > changes to a PR, use a change request! The NuttX reviewers are often too > nice > and only leave comments, or approve a PR but include some feedback in the > approval message. Using a change-request prevents the PR from getting > accidentally merged while the changes are pending, and that keeps the > quality > higher :) > > Best, > Matteo > >
