Ah here is the discussion sorry to reply in the vote thread first :-P 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. Merge rules. 1. Each change must be provided as PR that undergoes independent review process (see 1.16). 2. Single company / organization commit, review, and merge is not allowed. 3. Merge of PRs with unresolved discussions and "change request" marks is not allowed. 4. Self committed code merge with or without review is not allowed. 5. Direct push to master is not allowed. Breaking these rules will be punished. See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md Another story if contributors and maintainers adhere to the CG which seems a bigger issue at the moment.. we always have this punish part but this should be really last resort ;-) Tomek -- CeDeROM, SQ7MHZ, http://www.tomek.cedro.info On Tue, Nov 11, 2025 at 7:14 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
