+1 On Tue, 11 Nov 2025, 17:42 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 >
