Hm, I guess I just assumed this wasn't the case since it was being ignored. I don't think this vote is really necessary if we already have this rule in the guidelines. I guess the vote adds stricter wording, i.e. it is only up to the person who started the discussion to decide if it's resolved, no one else.
Thanks for pointing that out! Matteo On Tue, Nov 11, 2025, 9:30 PM Tomek CEDRO <[email protected]> wrote: > +1 in general for the idea :-) > > We already have this rule in Contributing Guidelines in 1.17.3 :-) > > https://github.com/apache/nuttx/blob/master/CONTRIBUTING.md#117-merge-rules > > 1.17.3. Merge of PRs with unresolved discussions and "change request" > marks is not allowed. > > Another story if contributors and maintainers adhere to the CG ;-) > > Tomek > > On Tue, Nov 11, 2025 at 5:43 PM 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 > > > > -- > CeDeROM, SQ7MHZ, http://www.tomek.cedro.info >
