Under Apache, nothing is really enforceable. No one person or group has any power and cannot compel anyone to do anything. Freedom is a core Apache value. ________________________________ From: Sebastien Lorquet <[email protected]> Sent: Wednesday, November 12, 2025 3:05 AM To: [email protected] <[email protected]> Subject: Re: [VOTE] Dismissing PR reviews - rules
At this point, it sounds like it would be desirable to have enforceable consequences for people that do not follow the rules. Sebastien On 11/12/25 04:36, Matteo Golin wrote: > 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 >>
