Hi all, here's my update on items 18 and 19 recently added. 18: +1 Following suggestions from my colleagues, please also add documentation to this.
19: +1 Makes sense to me. I strongly support this as we don't have that many people to review and approve. We can't stop going forward, but this item can be reviewed in the future, when more people are working in Nuttx. ________________________________ From: Tiago Medicci Serrano <tiago.medi...@gmail.com> Sent: Thursday, February 13, 2025 9:24 AM To: dev@nuttx.apache.org <dev@nuttx.apache.org> Subject: Re: [VOTE] NuttX Contributing Guidelines update 202502. [External: This email originated outside Espressif] My votes: 18. 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. +1. Documentation of a new feature should be provided in the same PR. Documentation is provided in the `nuttx` repository, so if the PR changes something that affects the current documentation, it should be treated at the same PR. 19. 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 (no > kernel/libs/upper-half drivers etc); > - It implements a new feature (or app) that doesn't introduce > any breaking changes or backward incompatibility; > - It didn't get the minimum of 4 reviewers after two weeks (to > be discussed); > - At least one independent reviewer reviewed it; > - It adheres to all other conditions. > The PR's author should: > - After a week (to be discussed) 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. +1. NuttX needs more features (new features) and more board/chip support. New features don't break any existing implementation and may be eligible for merging after at least one independent reviewer. Some notes on that taking *Zephyr* - our main competitor- as an example. In the last month, Zephyr had almost 400 authors and more than 2K commits ( https://github.com/zephyrproject-rtos/zephyr/pulse/monthly), but the PRs that implement new features don't require 4 reviewers to be merged. For example: - https://github.com/zephyrproject-rtos/zephyr/pull/85525 - https://github.com/zephyrproject-rtos/zephyr/pull/85374 - https://github.com/zephyrproject-rtos/zephyr/pull/85342 - https://github.com/zephyrproject-rtos/zephyr/pull/85284 NuttX had 57 authors and 344 commits ( https://github.com/apache/nuttx/pulse/monthly). This is 7x less authors/commits. We have limitations on what we can do trusting only on people. Exceptions like *19* should exist otherwise *NuttX will die as a distribution*. We should really focus our attention on automated testing (than being to rigid depending only on people for new features). Zephyr, with 7x more authors, doesn't rely on people that much. We really need more features and chip/board support and *Lazy Consensus* is important here. Em qui., 13 de fev. de 2025 às 05:15, Sebastien Lorquet < sebast...@lorquet.fr> escreveu: > you might be right, or not, I cant tell. > > To be honest, I find this specific rule to be very complex. it's not > easy to understand its effect (I did not apparently) > > so it should be reworded at least. > > I suggest still not to integrate it as-is. > > Sebastien > > > On 12/02/2025 19:21, Tiago Medicci Serrano wrote: > > Hi, > > > > Again, Sebastien, read it *carefully*. It seems you are not willing to do > > it. > > > > It doesn't bypass anything: > > > > - either the submitter still cares and will yell at people to get it > >> approved > > > > * My proposal says explicitly about this:* > > > > The PR's author should: > >> - After a week (to be discussed) without any reviewers, send > an > >> e-mail to the mailing list asking for more people to review it; > > > > *Continuing:* > > > > I dont want to see unsupervised auto commits. > > > > > >> The conditions listed in this point (no breakage, execution of tests, > >> etc) HAVE TO be verified. > > > > Neither do I! That's why there is the following rule: > > > > *The (required) independent reviewer* is responsible for > checking > >> if the PR matches the *Lazy Consensus* conditions and merging it > >> > > *THE INDEPENDENT REVIEWER *(not the PR's author) is responsible for > > checking and merging it. It requires *AT LEAST* one independent reviewer. > > > > So, if we were not able to have 4 reviewers *and* already asked for help. > > Then we can try to check if it's *eligible*. > > > > Em qua., 12 de fev. de 2025 às 15:10, Alan C. Assis <acas...@gmail.com> > > escreveu: > > > >> The goal is not to automatically merge PR, but only to avoid that some > PRs > >> that don't reach the maximum number of reviews be allowed to be merged. > >> > >> Typically, those who are most eager to impose new rules are not the > same as > >> those who are subject to them! > >> > >> BR, > >> > >> Alan > >> > >> > >> > >> > >> > >> > >> On Wed, Feb 12, 2025 at 2:55 PM Sebastien Lorquet <sebast...@lorquet.fr > > > >> wrote: > >> > >>> Again, I will vote against this. Not that it matters if a majority > wants > >>> to approve it. > >>> > >>> This is a bypass of all other rules we're trying to enforce. > >>> > >>> If such situation arise, there are two cases: > >>> > >>> - either the submitter still cares and will yell at people to get it > >>> approved > >>> > >>> - either it will be dropped entirely. > >>> > >>> I dont want to see unsupervised auto commits. > >>> > >>> The conditions listed in this point (no breakage, execution of tests, > >>> etc) HAVE TO be verified. > >>> > >>> Sebastien > >>> > >>> > >>> On 12/02/2025 17:14, Tiago Medicci Serrano wrote: > >>>> Hi! > >>>> > >>>> Thanks, Tomek ;) > >>>> > >>>> So, rewriting 19: > >>>> > >>>> *19.* 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 (no > >>>> kernel/libs/upper-half drivers etc); > >>>> - It implements a new feature (or app) that doesn't > >> introduce > >>> any > >>>> breaking changes or backward incompatibility; > >>>> - It didn't get the minimum of 4 reviewers after two weeks > >>> (to be > >>>> discussed); > >>>> - At least one independent reviewer reviewed it; > >>>> - It adheres to all other conditions. > >>>> The PR's author should: > >>>> - After a week (to be discussed) 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 qua., 12 de fev. de 2025 às 11:05, Tomek CEDRO <to...@cedro.info> > >>>> escreveu: > >>>> > >>>>> On Wed, Feb 12, 2025 at 2:13 PM Tiago Medicci Serrano > >>>>> <tiago.medi...@gmail.com> wrote: > >>>>>> Hi! > >>>>>> Tomek, there is an important missing point about 19 (that is part of > >> my > >>>>>> proposal): > >>>>>> > >>>>>> 19. "Lazy consensus" is a situation where PR is auto-merged into the > >>>>>>> upstream when not enough reviews are done in predefined time (i.e. > >>>>>>> there are no minimum required positive reviews within two weeks > >> time). > >>>>>>> PR should initially be treated according the general rules (4 > >>>>>>> independent reviewers); 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; > >>>>>>> After two weeks without any reviewers, we could merge if the above > >>>>>>> conditions are met and we have at least one independent reviewer. > >> This > >>>>>>> may solve situation when 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. > >>>>>> It's missing that a PR has to be *eligible *for requiring it and > this > >>>>>> includes some specific situations: > >>>>>> - It should not affect more than one chip/board; > >>>>>> - It applies to new features and apps that have no associated > >> breaking > >>>>>> changes and backward compatibility; > >>>>>> - It requires at least one independent reviewer; > >>>>>> - It requires an extensive testing section and proper documentation. > >>>>>> > >>>>>> These are *mandatory *conditions and we can vote it without it (this > >>> was > >>>>>> part of my considerations about *Lazy Consensus*). I would be > against > >>>>>> proposition 19 it if these requirements are missing. > >>>>>> > >>>>>> Can you please update proposition 19 and reconsider your vote based > >> on > >>>>> the > >>>>>> requirements? > >>>>> Hey there Tiago :-) > >>>>> > >>>>> This voting is to identify general rules that we all agree on > already. > >>>>> These points are supposed to be extract from our discussions. If I > >>>>> missed some key points please add them here folks! For proposed rules > >>>>> with doubts (0 or -1 votes) we should discuss more and update maybe > >>>>> even finally reject them. > >>>>> > >>>>> On weekend we will gather and sum up the current vote results. Then > >>>>> discussion will follow, where we may make clarifications, update > >>>>> doubted rules, and vote again. The goal is to have common agreement > on > >>>>> the rules update. What we want to add and what form this is all up to > >>>>> us. No one wants to enforce anything or create a monster that will > >>>>> make our work harder, just a tool to help in review and code quality > >>>>> improvement :-) > >>>>> > >>>>> If I provided incomplete information on proposition 19 then I am > >>>>> sorry! Tiago you are the author, please send updated proposition 19 > >>>>> text and note the old one is cancelled :-) > >>>>> > >>>>> Thanks!! :-) > >>>>> > >>>>> -- > >>>>> CeDeROM, SQ7MHZ, http://www.tomek.cedro.info > >>>>> >
