Hi, It is important to have a contingency in case a reviewer requests changes and then stops participating for whatever reason (for example, gets busy with other work in the meantime). So, that's good thinking to include item #2.
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. Are you sure you'd like to go straight to a [VOTE] without a [DISCUSS] thread first? Perhaps others may propose alternative contingency ideas, or offer thoughts on why reviewers should be able to dismiss other reviewers' change requests, so we can consider all aspects of this before we vote. Cheers, Nathan On Tue, Nov 11, 2025 at 11:43 AM Matteo Golin <[email protected]> wrote: > Hello everyone, > > I am proposing a vote for a new rule surrounding PR reviews for the NuttX > project. > > 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 :) > > -- > Matteo Golin >
