Hi, just an additional comment:
Lazy consensus is exactly what lead to absence of any testing and breakages.
This *Lazy consensus *is just about the timeframe and the lack of 4
reviewers. The PR should be compliant with all other requirements
(description, testing, documentation etc).
My proposal for this *Lazy consensus:*
14. Number of minimum required code review votes should be increased
> from 2 to 4. This will ensure cross-checks and filter out faulty
> changes.
14.1 A PR may be eligible to be merged under the concept of *Lazy
consensus* with the following conditions:
- It affects only a single chip or board;
- It didn't get the minimum of 4 reviewers after two weeks;
- It was reviewed by at least one independent reviewer;
- It adheres to all other conditions.
The PR's author should:
- After a week without any reviewers, send an e-mail to the
mailing list asking for more people to review it;
- Explain why the PR can't be split into smaller PRs (if
applicable);
- Ask for the independent reviewer to merge it after two weeks
without any other reviewers;
The (required) independent reviewer is responsible for checking if
the PR matches the Lazy Consensus conditions and merging it.
Em ter., 11 de fev. de 2025 às 11:05, Tiago Medicci Serrano <
tiago.medi...@gmail.com> escreveu:
> Hi!
>
> I agree with almost every single point from Sebastien and Tomek:
>
> Lazy consensus is exactly what lead to absence of any testing and
>> breakages.
>>
>
> This isn't entirely true. *Lazy consensus *is very bad if it affects a
> lot of people, but the Wi-Fi support for all Espressif chips was added with
> it (huge PR, few reviewers, doesn't affect any existing configuration).
> That's why I claim we should have exceptions.
>
> We talked about "lazy merge" already and there were opposing voices
>> (including me). The safe default and current updated approach is NOT
>> to merge unless author can prove its useful, working, tested, and top
>> priority does not break stuff. Good cross-check reviews / verification
>> is important.. even if it takes more time than expected. We should
>> value quality more over rushing code changes. This is where all recent
>> discussion came from. Am I wrong?
>
>
> I include myself on that too: I was opposed to it. But I've changed my
> mind about it when it meets this very specific conditions:
>
> 1. Code that affects only a single chip/vendor implementation;
> 2. Are hard to review because can't be split into smaller PRs;
>
> And, even the procedure for that should be well-documented:
>
> 1. PR should initially be treated according the general rules (4
> independent reviewers);
> 2. After a week without enough reviewers, a call should be made on the
> mailing list, explaining why the PR can't be split into smaller PRs;
> 3. After two weeks without any reviewers, we could merge if the above
> conditions are met and we have at least one independent reviewer;
>
> See, I prefer it to let PRs without any reviewers because we don't have
> enough people to review it (or we are not interested in that). It prevents
> people from forking the project just to be able to develop their stuff:
> *we* *really would not like that*. The PR's author is still responsible
> for fixing some bugs if found in the future.
>
> Please, consider this situation.
>
> Em ter., 11 de fev. de 2025 às 10:35, Tomek CEDRO <to...@cedro.info>
> escreveu:
>
>> On Tue, Feb 11, 2025 at 12:40 PM Tiago Medicci Serrano
>> <tiago.medi...@gmail.com> wrote:
>> > Two points that seem to be missing in the current *[VOTE] NuttX
>> > Contributing Guidelines update 202502.* thread:
>> >
>> > About commits...
>> >
>> > We were discussing splitting some changes into different PRs, adding the
>> > commit's message and description, etc but I couldn't find anything
>> (even on
>> > Docs) that mentions the rule of the commit being as small as it can be
>> *without
>> > breaking the build*.
>>
>> Thanks Tiago! We want to clearly state that breaking changes are not
>> welcome :-) True, we already require that each PR should be as small
>> as possible and change only one functional thing at time so we avoid
>> too-many-things-at-once bundles that are hard to analyze and verify..
>> but also cannot break things and sometimes bundles are exceptionally
>> acceptable.. I thought this is obvious.. but maybe we should add this
>> point :D
>>
>>
>> > About required reviewers...
>> >
>> > I have a new proposal about "lazy consensus".
>> >
>> > Currently, nothing being voted refers to it, but it may apply to one
>> > specific situation: there are huge PRs that can't be broken on smaller
>> > parts and affect only a single chip or board.
>> >
>> > For instance, this <https://github.com/apache/nuttx/pull/12004> PR
>> updated
>> > the wireless drivers for ESP32-S3. As can be seen, only a single
>> > independent reviewer reviewed it.
>> >
>> > And it makes sense that no one was willing to review it:
>> > - It's big (and can't be split into smaller PRs because the changes
>> depend
>> > on each other);
>> > - It affects only a single chip (and it's expected that no one other
>> than
>> > the manufacturer or the developer that submitted it is willing to review
>> > it);
>> >
>> > *In this specific case, if the PR adheres to the other guidelines
>> (proper
>> > testing, logs, documentation etc), we could rely on a minimum of 1 (one)
>> > independent reviewer and, after two weeks, merge it.*
>>
>> This way we should merge all stalled PRs with only one review right away
>> :-P
>>
>> We talked about "lazy merge" already and there were opposing voices
>> (including me). The safe default and current updated approach is NOT
>> to merge unless author can prove its useful, working, tested, and top
>> priority does not break stuff. Good cross-check reviews / verification
>> is important.. even if it takes more time than expected. We should
>> value quality more over rushing code changes. This is where all recent
>> discussion came from. Am I wrong?
>>
>> In case of pull 12004, I agree this is big, maybe should be split into
>> smaller PRs (i.e. why remove RMT support when updating WIFI?), maybe
>> build and runtime logs could be attached, but it was clearly stated
>> all these changes needs to be bundled not to break stuff, so it was
>> accepted, exceptional situations like this happen, but require
>> additional caution and testing. Still additional reviews can be easily
>> requested, right? It may take more time to verify but we will have
>> solid code base :-)
>>
>> I have ESP32S3 now. I can get others if needed. For instance I
>> recently also bough popular rpi-pico(-2-w) just to see it does not
>> build. I am open to vendors providing free devkits for testing
>> support. For instance STM is known to share their devkits for free,
>> maybe Espressif could provide some new stuff too? :-)
>>
>>
>> To sum up, we want to add additional point like this? Please update if
>> necessary :-)
>>
>> Pull Requests should be as small as possible and focused on only one
>> functional change. Different functional changes should be provided in
>> separate Pull Requests. Remember that breaking changes are not
>> welcome. Pull Requests must not break overall build, runtime, and
>> compatibility, especially for other components. When changes must be
>> bundled together in order to maintain functionality and
>> self-compatibility, exception can be made, and this must be clearly
>> stated there is no other way.
>>
>>
>> --
>> CeDeROM, SQ7MHZ, http://www.tomek.cedro.info
>>
>
