+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
