You're right, a DISCUSS thread would be a good idea! I suppose discussion shouldn't take place in a VOTE thread. I'll start one.
Best, Matteo On Tue, Nov 11, 2025 at 1:02 PM Nathan Hartman <[email protected]> wrote: > 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 > > >
