In the past we had a rule that no one should commit their own changes.  The 
reasoning behind that is that we didn't want people to make changes and commit 
them without community approval and concurrence.

One week is a proper amount of time to keep a change open for review (unless, 
as you mention, then is some critical issue depending on it).  When we joined 
Apache we were told that we should plan on two week turnarounds for changes.  
That is the average for Apache projects.

________________________________
From: Matteo Golin <[email protected]>
Sent: Tuesday, November 11, 2025 10:11 AM
To: [email protected] <[email protected]>
Subject: [DISCUSS] Dismissing PR reviews - rules

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

Reply via email to