+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

Reply via email to