I also don't understand this practice of separating documentation from code. When changes in the code have to be reflected in the documentation, what's the point of splitting it into separate commits? If, for example, I do a `git revert`, it may happen that the documentation and code are out of sync! The same applies to separate commits for arch and board, but in this case it's even worse, because we'll have commits that can't even be compiled. Now we'll have to pay extra attention to the changes that spread across multiple commits.
`git revert` and `git bisec` will be even more difficult, which is already problematic because we have to sync nuttx and nuttx-apps. I don't know who this rule is supposed to help, because it certainly isn't good for users and devs. pt., 7 mar 2025 o 10:27 Alin Jerpelea <jerpe...@gmail.com> napisał(a): > Hi Alan, > > development best practices ask for commits to be split in areas they touch > > Related to that PR: Documentation in not software and does not belong to > code changes in arch > > I think that this rule should be enforced even if we don't do LTS releases > for now but we will do them in the future. > > Best regards > > Alin > > On Fri, 7 Mar 2025, 09:58 Alan C. Assis, <acas...@gmail.com> wrote: > > > Hi Tomek, > > > > My vote -1 was because the breaking text was saying that the author of > > commit needs to test it and many (all?) arch and boards supported by > NuttX. > > > > This transfers the responsibility of NuttX QA from the project > organization > > to the end developer, that is unfair. We as project responsible need to > > improve or CI and hw testing to detect possible commits that could break > > the system. > > > > About the PRs, one I missed the change request and the other the author > had > > fixed all important questions, the only remaining has separating the > > Documentation into a new PR, but this rules doesn't need to apply because > > we are not going to LTS now. > > > > BR, > > > > Alan > > > > On Thu, Mar 6, 2025 at 4:13 PM Tomek CEDRO <to...@cedro.info> wrote: > > > > > Alan, I think that some honest discussion may be necessary regarding > > > rules propositions that are blocked only with your -1 vote and quite > > > against the discussion on breaking changes, thus allowing breaking > > > code to be freely merged with the upstream, with no breaking changes > > > handling process, and by single company / group of interests. We also > > > noticed you have recently merged two PRs with pending "change > > > requested" discussions, what seems even against the old and general > > > merge rules. The whole breaking changes discussion was to avoid this > > > kind of situations. > > > > > > My question is do you still block rules 10 (breaking changes not > > > welcome), 12 (breaking rules handling process), 16 (single company > > > commit/review/merge)? If so what are your propositions in this area? > > > How do you see these points exactly full text? > > > > > > We really need to fix these areas as people are loosing trust to NuttX > > > and I understand that. > > > > > > Thanks :-) > > > Tomek > > > > > > > > > ### 10. Breaking changes not welcome. > > > > > > Breaking changes are not welcome. We do not "break by design". When > > > unavoidable breaking changes need prior discussion and agreement of > > > the community (see Breaking Changes handling rule). This is anything > > > that alters Build / Kernel / Architecture / API, alters both nuttx and > > > nuttx-apps repo at the same time, breaks build/runtime/api for single > > > or many boards/architectures/applications, breaks self-compatibility, > > > breaks build/runtime compatibility with existing release code > > > (packages) both for nuttx and nuttx-apps, etc. Because thousands of > > > users / companies and their projects / products depend on NuttX code, > > > we strongly prefer self-compatibility and long-term maintenance over > > > "change is good" ideologies. Any code change may impact other users > > > and their business, please keep that in mind. > > > > > > See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > In the internet form you gave -1 and 0 answers, with priority to > > > answer -1, thus the only blocking vote for breaking changes. > > > > > > Remarks: > > > Alan: -1, Breaking are necessary sometimes, saying "Breaking changes > > > are not welcome" will make people afraid of contributing innovation > > > that breaks existing APIs. > > > > > > > > > ### 12. Breaking changes handling process. > > > > > > This rule complements "Breaking changes not welcome" rules. We avoid > > > breaking changes unless absolutely necessary and unavoidable (i.e. > > > security fix, broken code, etc), then special case considerations may > > > apply: > > > 1. First reviewer that recognizes a breaking change should block > > > accidental merge with "Request Changes" mark and ask for discussion. > > > 2. PR is marked as "Draft" to avoid accidental merge. > > > 3. Detailed discussion should follow both in PR AND dev@ Mailing List. > > > 4. Alternative non-breaking alternative solution is researched with > > > help of the community. > > > 5. Breaking change after discussion / updates is voted on the mailing > > > list, requires at least 4 +1 binding votes and single -1 binding vote > > > blocks the change (binding vote means PMC member). > > > 6. Breaking changes **must** be verified on various different real > > > world hardware architectures, build and runtime logs are > > > **mandatory**, help of the community is desired. > > > 7. Breaking change requires at least 4 independent organizations > > > positive PR reviews. > > > 8. Change must be well documented (buid/runtime test logs, pr, git > > > commit, documentation, release notes, etc). > > > 9. Change must be clearly marked with [BREAKING] mark (pr, git commit, > > > release notes, etc). > > > > > > See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > Here too -1 and 0 was given, the only -1. > > > > > > Remarks: > > > Alan: -1, Mostly a repetition of 10 mixed with 9 and 11 > > > > > > > > > ### 16. Self company commit/review/merge not allowed > > > > > > Single company commit, review, merge is not allowed. Each PMC Member, > > > Committer, and Reviewer must report up-to-date Affiliation for clear > > > identification. > > > > > > See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > Also -1 and 0 answers given here with priority to -1, thus the only -1 > > > vote. > > > > > > Remarks: > > > Alan: -1, People in big companies could work in different areas or be > > > physically distant of the author of PR, and sometimes someone in the > > > company knows more about that subject than some "independent" > > > reviewer. > > > > > > > > > > > > > > > > > > On Sat, Mar 1, 2025 at 11:11 AM Alan C. Assis <acas...@gmail.com> > wrote: > > > > > > > > Hi Tomek, > > > > > > > > Nice work! Kudos! > > > > > > > > About Documentation I voted +1 to have Documentation, but later I > > > realized > > > > it was required to be in a separate PR, which I don't agree with. > > > > > > > > When I contributor submit a new feature that doesn't have > > Documentation I > > > > ask them to include it to avoid creating hidden feature (a nice > > > > functionality that nobody knows what is it used for and how to use > it, > > I > > > > have a nice example here: > > > > > > > > > > https://acassis.wordpress.com/2024/07/02/how-a-supposed-malfunction-revealed-another-hidden-feature-of-nuttx/ > > > > ). > > > > > > > > NuttX has many hidden features, this is something we need to fix, > more > > > and > > > > better documentation will help newcomers to use the system. > > > > > > > > Even basic features like USERLEDS are missing documentation, people > > spend > > > > days before they realize they need to include > > > > userled_lower_initialize("/dev/userleds") into bringup. There is no > > > > reference to it in our site: > > > > > > > > > > https://nuttx.apache.org/docs/latest/search.html?q=userled_lower_initialize&check_keywords=yes&area=default > > > > > > > > The only reference to LEDs is the WS2812: > > > > > > > > > > https://nuttx.apache.org/docs/latest/components/drivers/character/leds/index.html > > > > > > > > BR, > > > > > > > > Alan > > > > > > > > On Sat, Mar 1, 2025 at 4:00 AM Tomek CEDRO <to...@cedro.info> wrote: > > > > > > > > > My thoughts and comments that I did not want to put as part of the > > > > > results message: > > > > > > > > > > It seems like this voting revealed two mindsets - one wants quick > and > > > > > dirty experimental changes with low bar for acceptance that may be > > > > > streamed up from a single big organization that is probably paid > for > > > > > the amount of changes or can simply allow to buy that much focus, > as > > > > > opposed to more careful approach where self-compatibility and long > > > > > term maintenance are more important than quickly changing features > > > > > because of most probable personal and financial responsibility for > > > > > unexpected additional maintenance and maybe even damages (small > > > > > companies). This may look like Linux vs BSD, maybe more general > terms > > > > > progressive vs conservative. NuttX was initially BSD, that aligned > > > > > with my mindset, and most part of the community seems quite > > > > > conservative that is opposing enforced changes. > > > > > > > > > > If we want to follow other moving-target projects that try to catch > > > > > some sort of rabbit all the time, just to gather some new > community, > > > > > then we will loose existing community that was here all the time > just > > > > > to avoid that rabbit. In that case no tools will be helpful or even > > > > > necessary because these will always prove only "current things" as > > > > > bleeding edge is in the mindset and you cannot ever fix "break by > > > > > design". Those two worlds are exclusive. > > > > > > > > > > If we ever agree internally to a moving target approach, and have > no > > > > > commitment to self-compatibility and long term maintenance as the > > > > > ultimate goal, then NuttX will become just like any other project > > that > > > > > you will have to re-discover each time you use it after update, and > > > > > there will be completely no difference which one to choose. > > > > > > > > > > I did my best to clarify things. > > > > > > > > > > Have a good weekend folks :-) > > > > > Tomek > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sat, Mar 1, 2025 at 7:23 AM Tomek CEDRO <to...@cedro.info> > wrote: > > > > > > > > > > > > Allright Ladies and Gentlemen, here goes the results :-) > > > > > > > > > > > > ACCEPTED: > > > > > > 1. Contributing Guidelines with hints for Reviewers. > > > > > > 2. PR and GIT COMMITS must adhere to Contributing Guidelines or > > > rejected. > > > > > > 3. Git commit messages as important as PR description. > > > > > > 4. Proper description details requirements. > > > > > > 5. PR must adhere to description requirements. > > > > > > 6. Git commit message must adhere to description requirements. > > > > > > 7. Git commit message mandatory fields (topic, desctiption, > > > signature). > > > > > > 9. Zero trust approach to user testing. > > > > > > 11. Respect long term maintenance and self-compatibility. > > > > > > 13. Breaking changes build and runtime test logs are mandatory. > > > > > > 14. Minimum code reviews. > > > > > > 17. Merge rules. > > > > > > 18. PR as small as possible. > > > > > > > > > > > > REJECTED: > > > > > > 8. Changes must come with documentation. > > > > > > 10. Breaking changes not welcome. > > > > > > 16. Self company commit/review/merge not allowed > > > > > > 19. Lazy Consensus. > > > > > > > > > > > > INCOMPLETE: > > > > > > 12. Breaking changes handling process. > > > > > > 15. Reviews independence. > > > > > > > > > > > > > > > > > > Please verify the answers, it took me ~3h of manual responses > > > processing > > > > > ;-) > > > > > > > > > > > > I will next prepare a PR draft with updated guidelines for final > > > > > > polishing and review. In addition to points Accepted I will also > > add > > > > > > points Rejected/Incomplete marked for discussion and hopefully we > > > will > > > > > > reach the agreement on independence, breaking changes, quality, > > > trust, > > > > > > compatibility, and maintenance. > > > > > > > > > > > > Thanks! :-) > > > > > > Tomek > > > > > > > > > > > > > > > > > > ### 1. Contributing Guidelines with hints for Reviewers. > > > > > > > > > > > > We are adding additional section for Reviewers to Contributing > > > > > > Guidelines in order to provide checklist and complementary set of > > > > > > rules that should filter out breaking code as much as possible > also > > > on > > > > > > our side. > > > > > > > > > > > > +1 Tomek / PMC > > > > > > +1 Alin / PMC > > > > > > +1 Tiago / PMC > > > > > > +1 TimK > > > > > > +1 Lup / PMC > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan / PMC > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 13 (6 binding), 0: 0, -1: 0. > > > > > > > > > > > > Remarks: > > > > > > TimH: +1 to all - on the basis that if this doesn't work out > quite > > > > > > right it will be reviewed and changed. Best to try something like > > > this > > > > > > than not - it will become clear very soon if something isn't > > working > > > > > > as intended! > > > > > > > > > > > > > > > > > > > > > > > > ### 2. PR and GIT COMMITS must adhere to Contributing Guidelines > or > > > > > rejected. > > > > > > > > > > > > Each PR and GIT COMMIT **must** adhere to requirements presented > in > > > > > > Contributing Guidelines or will be auto-rejected until fixed / > > > > > > updated. Both code authors and reviewers/committers must follow > the > > > > > > rules. Special cases are defined in a separate dedicated rules. > > > > > > > > > > > > +1 Tomek > > > > > > +1 Alin > > > > > > +1 Tiago > > > > > > +1 TimK > > > > > > +1 Lup > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 13 (6 binding), 0: 0, -1: 0. > > > > > > > > > > > > Remarks: none. > > > > > > > > > > > > > > > > > > > > > > > > ### 3. Git commit messages as important as PR description. > > > > > > > > > > > > Git commit messages are as important as PR descriptions. These > > > provide > > > > > > in-code descriptions of each change and are git interface > > > independent. > > > > > > > > > > > > +1 Tomek > > > > > > +1 Alin > > > > > > +1 Tiago > > > > > > +1 TimK > > > > > > +1 Lup > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 13 (6 binding), 0: 0, -1: 0. > > > > > > > > > > > > Remarks: none. > > > > > > > > > > > > > > > > > > > > > > > > ### 4. Proper description details requirements. > > > > > > > > > > > > Proper description of change is mandatory. Description must > contain > > > > > > explanation on what proposed change do, why it is necessary, what > > if > > > > > > fixes, and how things are changed / fixed / updated, what is the > > > > > > impact (build / runtime / api / what area), how it was tested. > > Local > > > > > > code build and real world hardware runtime test logs must be > > provided > > > > > > (for code related changes). Description can be single..several > > > > > > sentences long or bullet points but enough for anyone to > understand > > > > > > change goals and details. Usually it will look similar for PR and > > git > > > > > > commit message. > > > > > > > > > > > > +1 Tomek > > > > > > +1 Alin > > > > > > +1 Tiago > > > > > > +1 TimK > > > > > > +1 Lup > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 13 (6 binding), 0: 0, -1: 0. > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Tomek: +1, However I understand this PR template is separate from > > the > > > > > > rule and will be updated / voted independently. > > > > > > > > > > > > TimK: +1, Better to have the 'Motivation/Background' section, > while > > > > > > simplifying the rest. In my view, commit messages should address > > the > > > > > > 'What', whereas PR documents should elaborate more on the 'Why'. > > > > > > > > > > > > > > > > > > > > > > > > ### 5. PR must adhere to description requirements. > > > > > > > > > > > > Proper description in PR according to template is mandatory, fill > > in > > > > > > all required fields or change is auto-rejected until fixed / > > > updated. > > > > > > For code changes build and runtime logs are mandatory to prove > code > > > > > > was tested on at least one real world hardware. > > > > > > > > > > > > +1 Tomek > > > > > > +1 Alin > > > > > > +1 Tiago > > > > > > +1 TimK > > > > > > +1 Lup > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 13 (6 binding), 0: 0, -1: 0. > > > > > > > > > > > > Remarks: This point was mostly missing in the answers but no 0 or > > -1 > > > > > > reported so assuming +1, as voted before, and the template > example > > is > > > > > > taken from existing Guidelines, will discuss the PR. > > > > > > > > > > > > > > > > > > PR TEMPLATE / EXAMPLE: > > > > > > > > > > > > Summary > > > > > > 1. Why change is necessary (fix, update, new feature)? > > > > > > 2. What functional part of the code is being changed? > > > > > > 3. How does the change exactly work (what will change and how)? > > > > > > 4. Related NuttX Issue reference if applicable. > > > > > > 5. Related NuttX Apps Issue / Pull Request reference if > applicable. > > > > > > 6. Related NuttX Documentation Pull Request reference if > > applicable. > > > > > > > > > > > > Impact > > > > > > 1. New feature added? Existing feature changed? > > > > > > 2. User (will user need to adapt to change)? NO / YES (please > > > describe > > > > > if yes). > > > > > > 3. Build (will build process change)? NO / YES (please descibe if > > > yes). > > > > > > 4. Hardware (will arch(s) / board(s) / driver(s) change)? NO / > YES > > > > > > (please describe if yes). > > > > > > 5. Documentation (is update required / provided)? NO / YES > (please > > > > > > describe if yes). > > > > > > 6. Security (any sort of implications)? NO / YES (please describe > > if > > > > > yes). > > > > > > 7. Compatibility (backward/forward/interoperability)? NO / YES > > > (please > > > > > > describe if yes). > > > > > > 8. Anything else to consider? > > > > > > > > > > > > Testing > > > > > > > > > > > > 1. I confirm that changes are verified on local setup and works > as > > > > > > intended NO / YES. > > > > > > 2. Build Host(s): OS (Linux,BSD,macOS,Windows,..), > > > CPU(Intel,AMD,ARM), > > > > > > compiler(GCC,CLANG,version), etc. > > > > > > Target(s): arch(sim,RISC-V,ARM,..), board:config, etc. > > > > > > 3. Testing logs before change: > > > > > > runtime / build logs before change goes here > > > > > > 4.Testing logs after change: > > > > > > runtime / build logs after change goes here > > > > > > 5. (optional) How to repeat. You can also provide steps on how to > > > > > > reproduce problem and verify the change if not obvious from test > > > logs. > > > > > > > > > > > > Optional PR remarks: > > > > > > 1. This PR introduces only one functional change. > > > > > > 2. I have updated all required description fields above. > > > > > > 3. My PR adheres to Contributing Guidelines and Documentation > (git > > > > > > commit message, coding standard, testing etc). > > > > > > 4. My PR is still work in progress (not ready for review). > > > > > > 5. My PR is ready for review and can be safely merged into a > > > codebase. > > > > > > > > > > > > > > > > > > > > > > > > ### 6. Git commit message must adhere to description > requirements. > > > > > > > > > > > > Proper GIT COMMIT message according to template is mandatory, or > > > > > > change is rejected until fixed / updates. Build and runtime logs > > are > > > > > > optional here if these are too long and already provided in PR. > > > > > > > > > > > > Git commit message consists of: > > > > > > 1. Topic with functional name prefix, ":" mark, and short > > > > > > self-explanatory context. > > > > > > 2. Blank line > > > > > > 3. Description on what is changed, how, and why. May use several > > > > > > lines, short sentences, or bullet points. > > > > > > 4. Blank line. > > > > > > 5. Signature (created with `git commit -s`). > > > > > > > > > > > > GIT COMMIT TEMPLATE / EXAMPLE: > > > > > > > > > > > > net/can: Add g_ prefix to can_dlc_to_len and len_to_can_dlc. > > > > > > > > > > > > Add g_ prefix to can_dlc_to_len and len_to_can_dlc to > > > > > > follow NuttX coding style conventions for global symbols, > > > > > > improving code readability and maintainability. > > > > > > * you can also use bullet points. > > > > > > * to note different thing briefly. > > > > > > > > > > > > +1 Tomek > > > > > > +1 Alin > > > > > > +1 Tiago > > > > > > +1 TimK > > > > > > +1 Lup > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 13 (6 binding), 0: 0, -1: 0. > > > > > > > > > > > > Remarks: > > > > > > > > > > > > TimK: +1, Regarding the commit message header, I recommend using > > the > > > > > > style adopted by the Angular Community, which is widely accepted. > > > > > > <type>(<scope>): <short summary> > > > > > > > > > > > > Filipe: +1, Let's make sure we have example for this on docs and > > also > > > > > > on PR bot). > > > > > > > > > > > > > > > > > > > > > > > > ### 7. Git commit message mandatory fields (topic, desctiption, > > > > > signature). > > > > > > > > > > > > Each git commit message must consist of topic, description, and > > > > > > signature (git commit -s), as presented in GIT COMMIT TEMPLATE, > > which > > > > > > are mandatory, or change is auto-rejected until fixed / updated. > > > > > > > > > > > > +1 Tomek > > > > > > +1 Alin > > > > > > +1 Tiago > > > > > > +1 TimK > > > > > > +1 Lup > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 13 (6 binding), 0: 0, -1: 0. > > > > > > > > > > > > Remarks: none. > > > > > > > > > > > > > > > > > > > > > > > > ### 8. Changes must come with documentation. > > > > > > > > > > > > Changes must come with with documentation update where > applicable. > > > For > > > > > > maintenance reasons code and documentation should be split into > two > > > > > > separate PR with the same name marked [1/2 CODE] for code and > [2/2 > > > > > > DOC] for documentation. If change presents new functionality a > > > > > > documentation must be provided along with the code (not in > future). > > > If > > > > > > change requires documentation update it must be contained along > > with > > > > > > the code (not in future). Successful documentation build log > > shortcut > > > > > > is welcome. > > > > > > > > > > > > See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > > > > > > > 0: Tomek / PMC > > > > > > 0 Alin / PMC > > > > > > 0 Tiago / PMC > > > > > > -1 TimK > > > > > > 0 Lup / PMC > > > > > > -1 Filipe > > > > > > -1 Sebastien > > > > > > -1 Nathan / PMC > > > > > > 0 Dmitri > > > > > > -1 Matteo > > > > > > 0 Roberto > > > > > > +1 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: REJECTED / +1: 2 (1 binding), 0: 6 (4 binding), -1: 5 (1 > > > > > binding). > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Tomek: 0, Having documentation in a single PR (same pr separate > > > commit > > > > > > is easier to perform and review, otherwise we may get out of > > code/doc > > > > > > sync? But if this is the only way and better for release manager > > then > > > > > > okay. > > > > > > > > > > > > Alin: 0, Having documentation in a single PR (same pr separate > > commit > > > > > > is easier to perform and review. The release process can use it. > > > > > > > > > > > > Tiago: 0, Yes, documentation should be provided, but I don't see > > any > > > > > > reason for splitting it into two different PRs. We keep our > > > > > > documentation in the same repository and - for the sake of > > > > > > traceability - it should be updated in the same PR (separate > > commit, > > > > > > not PR). We should make reviewers' and committers lives easier. > > > > > > Alternative writing would be: > > > > > > "Changes must come with a documentation update where applicable. > > For > > > > > > maintenance reasons, code and documentation should be split into > > two > > > > > > commits in the same PR. If change presents new functionality > > > > > > documentation must be provided along with the code (not in the > > > > > > future). If change requires a documentation update it must be > > > > > > contained along with the code (not in the future)." > > > > > > > > > > > > TimK: -1, I'd like say "should" instead of "must". > > > > > > > > > > > > Lup: 0, It depends? Smaller PRs can include a Doc Commit. When I > > add > > > a > > > > > > new Arch + Board (e.g. StarPro64), the PR will include a link to > my > > > > > > article that explains the new code. Then I prepare another PR for > > the > > > > > > User Docs. > > > > > > > > > > > > Filipe: -1, Don't see any problem in having documentation on same > > PR, > > > > > > in fact I think it makes things easier. > > > > > > > > > > > > Sebastien: -1, separate commits in same pr. > > > > > > > > > > > > Nathan: -1, : Similar to others' comments. Also, could be same PR > > but > > > > > > separate commits? > > > > > > > > > > > > Matteo: -1, : Documentation should be required, in the same PR as > > the > > > > > > change. No separate PRs. > > > > > > > > > > > > > > > > > > PROPOSED UPDATED TEXT: > > > > > > > > > > > > Changes must come with a documentation update where applicable. > For > > > > > > maintenance reasons, code and documentation should be split into > > two > > > > > > commits in the same PR. If change presents new functionality > > > > > > documentation must be provided along with the code (not in the > > > > > > future). If change requires a documentation update it must be > > > > > > contained along with the code (not in the future). Successful > > > > > > documentation build log shortcut is welcome. > > > > > > > > > > > > See: > > > > > > 1. https://github.com/apache/nuttx/tree/master/Documentation > > > > > > 2. https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > > > > > > > > > > > > > <--- Quick note from Tomek: TimK the word "must" is used on > purpose > > > to > > > > > > improve documentation that is usually skipped, and complemented > > > "where > > > > > > applicable" :-) > > > > > > > > > > > > > > > > > > > > > > > > ### 9. Zero trust approach to user testing. > > > > > > > > > > > > We implement zero trust approach to user provided testing. It is > > the > > > > > > commit author duty to provide real world hardware build and > runtime > > > > > > logs for at least one device. Remember that any code change may > > break > > > > > > things for others, please avoid that. > > > > > > > > > > > > See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > > > > > > > +1 Tomek / PMC > > > > > > +1 Alin / PMC > > > > > > +1 Tiago / PMC > > > > > > +1 TimK > > > > > > +1 Lup / PMC > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan / PMC > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > 0 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 12 (5 binding), 0: 1 (1 binding), -1: 0. > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Alan: 0, We cannot transfer this responsibility to the developer, > > he > > > > > > should test it in the HW he has, but we need to have better HW > > > > > > coverage to avoid issues. > > > > > > > > > > > > > > > > > > > > > > > > ### 10. Breaking changes not welcome. > > > > > > > > > > > > Breaking changes are not welcome. We do not "break by design". > When > > > > > > unavoidable breaking changes need prior discussion and agreement > of > > > > > > the community (see Breaking Changes handling rule). This is > > anything > > > > > > that alters Build / Kernel / Architecture / API, alters both > nuttx > > > and > > > > > > nuttx-apps repo at the same time, breaks build/runtime/api for > > single > > > > > > or many boards/architectures/applications, breaks > > self-compatibility, > > > > > > breaks build/runtime compatibility with existing release code > > > > > > (packages) both for nuttx and nuttx-apps, etc. Because thousands > > of > > > > > > users / companies and their projects / products depend on NuttX > > code, > > > > > > we strongly prefer self-compatibility and long-term maintenance > > over > > > > > > "change is good" ideologies. Any code change may impact other > users > > > > > > and their business, please keep that in mind. > > > > > > > > > > > > See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > > > > > > > +1 Tomek / PMC > > > > > > +1 Alin / PMC > > > > > > +1 Tiago / PMC > > > > > > +1 TimK > > > > > > +1 Lup / PMC > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan / PMC > > > > > > +1 Dmitri > > > > > > 0 Matteo > > > > > > +1 Roberto > > > > > > -1 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: REJECTED / +1: 11 (5 binding), 0: 1 , -1: 1 (1 binding). > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Alan: -1, Breaking are necessary sometimes, saying "Breaking > > changes > > > > > > are not welcome" will make people afraid of contributing > innovation > > > > > > that breaks existing APIs. > > > > > > > > > > > > <--- Quick note from Tomek: Alan please re-read that point, it > was > > > > > > updated not to scare people except people that want to break the > > code > > > > > > on purpose, a dedicated section is mentioned and created for > > breaking > > > > > > changes after last round. The whole point of this voting is to > > avoid > > > > > > breaking changes that impact self-compatibility and long term > > > > > > maintenance. It is again blocked without preferred text > > alternative. > > > > > > You gave -1 answer then 0 and asked to use the first form, > correct? > > > > > > > > > > > > > > > > > > > > > > > > ### 11. Respect long term maintenance and self-compatibility > > > > > > > > > > > > We respect long term maintenance and self-compatibility is our > > > > > > ultimate goal. Alternative solutions and non-invasive approaches > > are > > > > > > preferred that offers user a choice and compatibility. Breaking > > > > > > changes are avoided, and planned towards next major release, see > > > > > > Breaking Changes rule. > > > > > > Experimental code that does not impact overall project > > > > > > self-compatibility in terms of Breaking Changes should be clearly > > > > > > marked [EXPERIMENTAL]. > > > > > > > > > > > > See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > > > > > > > > > > > > > +1 Tomek / PMC > > > > > > +1 Alin / PMC > > > > > > +1 Tiago / PMC > > > > > > +1 TimK > > > > > > +1 Lup / PMC > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan / PMC > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > 0 Roberto > > > > > > 0 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 11 (5 binding), 0: 2 (1 binding), -1: 0. > > > > > > > > > > > > Remarks: none. > > > > > > > > > > > > > > > > > > > > > > > > ### 12. Breaking changes handling process. > > > > > > > > > > > > This rule complements "Breaking changes not welcome" rules. We > > avoid > > > > > > breaking changes unless absolutely necessary and unavoidable > (i.e. > > > > > > security fix, broken code, etc), then special case considerations > > may > > > > > > apply: > > > > > > 1. First reviewer that recognizes a breaking change should block > > > > > > accidental merge with "Request Changes" mark and ask for > > discussion. > > > > > > 2. PR is marked as "Draft" to avoid accidental merge. > > > > > > 3. Detailed discussion should follow both in PR AND dev@ Mailing > > > List. > > > > > > 4. Alternative non-breaking alternative solution is researched > with > > > > > > help of the community. > > > > > > 5. Breaking change after discussion / updates is voted on the > > mailing > > > > > > list, requires at least 4 +1 binding votes and single -1 binding > > vote > > > > > > blocks the change (binding vote means PMC member). > > > > > > 6. Breaking changes **must** be verified on various different > real > > > > > > world hardware architectures, build and runtime logs are > > > > > > **mandatory**, help of the community is desired. > > > > > > 7. Breaking change requires at least 4 independent organizations > > > > > > positive PR reviews. > > > > > > 8. Change must be well documented (buid/runtime test logs, pr, > git > > > > > > commit, documentation, release notes, etc). > > > > > > 9. Change must be clearly marked with [BREAKING] mark (pr, git > > > commit, > > > > > > release notes, etc). > > > > > > > > > > > > See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > > > > > > > +1 Tomek / PMC > > > > > > +1 Alin / PMC > > > > > > +1 Tiago / PMC > > > > > > +1 Lup / PMC > > > > > > .. <- votes lost here > > > > > > 0 Roberto > > > > > > -1 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: REJECTED / INCOMPLETE / +1: 5 (4 binding), 0: 1, -1: (1 > > > binding). > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Alan: -1, Mostly a repetition of 10 mixed with 9 and 11. > > > > > > > > > > > > <--- Quick note from Tomek: Alan, this procedure is dedicated for > > > > > > handling breaking changes. If rejecting please provide > alternative > > > > > > expected text. > > > > > > > > > > > > > > > > > > > > > > > > ### 13. Breaking changes build and runtime test logs are > mandatory. > > > > > > > > > > > > Breaking changes are special case where build and runtime test > logs > > > > > > (i.e. apps/ostest) from more than one different architecture is > > > > > > **mandatory** . QEmu tests does not count here as it passed > > breaking > > > > > > change that did not work on a real hardware. Community support is > > > > > > desired. > > > > > > > > > > > > See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > > > > > > > +1 Tomek > > > > > > +1 Alin > > > > > > +1 Tiago > > > > > > +1 TimK > > > > > > +1 Lup > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > 0 Roberto > > > > > > 0 Alan > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 11 (5 binding), 0: 2 (1 binding), -1: 0. > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Sebastien: +1, Also some mandatory documentation on how to fix > the > > > > > > build after the breaking change. rust has cargo fix. Python has > > 2to3. > > > > > > > > > > > > Alan: 0, All the changes need to be equally coveraged, not only > > those > > > > > > that break some existing code > > > > > > > > > > > > > > > > > > > > > > > > ### 14. Minimum code reviews. > > > > > > > > > > > > Each PR requires at least 2 independent positive reviews, except > > > > > > Breaking Changes where at least 4 positive independent > > organizations > > > > > > reviews, are required before merge to the upstream. > > > > > > > > > > > > +1 Tomek / PMC > > > > > > +1 Alin / PMC > > > > > > +1 Tiago / PMC > > > > > > -1 TimK > > > > > > +1 Lup / PMC > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > 0 Nathan / PMC > > > > > > 0 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 10 (5 binding), 0: 2 (1 binding), -1: 1. > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Tomek: +1, Although I think 3 should be default to increase > > > cross-checks. > > > > > > > > > > > > Alin: +1, minimum 3. > > > > > > > > > > > > Tiago: +1, I still prefer Nathan's proposal of creating "areas". > > > > > > Documentation and experimental features shouldn't require 2 > > > reviewers. > > > > > > For the sake of simplicity, this rule works. Even 2 reviewers for > > > > > > documentation and experimental features are too restrictive. > > > > > > > > > > > > TimK: -1, "at least 2 independent positive reviews", may be too > > high > > > bar > > > > > we set. > > > > > > > > > > > > Nathan: 0, Tiago summarized my proposal above. However, I am OK > > with > > > > > > whatever the community decides on this. > > > > > > > > > > > > Matteo: +1, Agree that pure documentation changes can have 1 > > > reviewer. > > > > > > > > > > > > <--- Quick note from Tomek: We currently require 2 independent > > > > > > reviewers right now for all changes, when breaking changes are > > > > > > reported or discovered then 4 reviewers are required. Its not a > big > > > > > > change but simple and should prevent breaking changes. Nathan's > > > > > > proposal is good and desired, we will for sure make it in another > > > > > > step. > > > > > > > > > > > > > > > > > > > > > > > > ### 15. Reviews independence. > > > > > > > > > > > > PR Reviews should come from independent organizations. Each PMC > > > > > > Member, Committer, and Reviewer must report up-to-date > Affiliation > > > for > > > > > > clear identification. When code comes from the same organization > as > > > > > > positive review, then at least one independent review is > necessary > > > > > > (except Breaking Changes). Self review is not allowed. > > > > > > > > > > > > See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > > > > > > > > > > > > > +1 Tomek / PMC > > > > > > +1 Alin / PMC > > > > > > +1 Tiago / PMC > > > > > > +1 TimK > > > > > > +1 Lup / PMC > > > > > > +1 Filipe > > > > > > -1 Sebastien > > > > > > ?? Nathan / PMC > > > > > > 0 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > 0 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: INCOMPLETE / +1: 9 (4 binding), 0: 2 (1 binding), -1: 1. > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Sebastien: -1, No. Reviewers must not be from same organization > as > > > > > > coder. Otherwise there is no independence. > > > > > > > > > > > > Nathan: ??, Reviews from same organization are nice to have, but > > > > > > shouldn't count toward number of reviews needed. > > > > > > > > > > > > <--- Quick note from Tomek: This is why I preferred online > testing > > > > > > solution because there _must_ be a valid vote cast, also proposed > > > > > > alternative text was required ;-) This point goal was to solve > > single > > > > > > company pr/review/merge. Please provide alternative text that you > > > > > > expect! :-) > > > > > > > > > > > > > > > > > > > > > > > > ### 16. Self company commit/review/merge not allowed > > > > > > > > > > > > Single company commit, review, merge is not allowed. Each PMC > > Member, > > > > > > Committer, and Reviewer must report up-to-date Affiliation for > > clear > > > > > > identification. > > > > > > > > > > > > See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > > > > > > > > > > > > > +1 Tomek / PMC > > > > > > +1 Alin / PMC > > > > > > +1 Tiago / PMC > > > > > > +1 TimK > > > > > > +1 Lup / PMC > > > > > > +1 Filipe > > > > > > 0 Sebastien > > > > > > +1 Nathan / PMC > > > > > > 0 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > -1 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: REJECTED / +1: 10 (5 binding), 0: 2, -1: 1 (1 binding). > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Alan: -1, People in big companies could work in different areas > or > > be > > > > > > physically distant of the author of PR, and sometimes someone in > > the > > > > > > company knows more about that subject than some "independent" > > > > > > reviewer. > > > > > > > > > > > > <--- Quick note from Tomek: This point 16 can be merged with 15. > > > Again > > > > > > the goal was to get independent reviews. Please provide full > > > > > > alternative text in comments. > > > > > > > > > > > > > > > > > > > > > > > > ### 17. Merge rules. > > > > > > > > > > > > Each change **must** be provided as PR that undergoes independent > > > > > > review process. Self committed code merge with or without review > is > > > > > > not allowed, just as direct push to master, and will be punished. > > > > > > > > > > > > +1 Tomek > > > > > > +1 Alin > > > > > > +1 Tiago > > > > > > +1 TimK > > > > > > +1 Lup > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > 0 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 12 (5 binding), 0: 1 (1 binding), -1: 0. > > > > > > > > > > > > Remarks: > > > > > > > > > > > > TimK: +1, However, I would prefer the maintainer to perform a > > 'squash > > > > > > merge' by default. In the case of a significant or breaking PR > > > change, > > > > > > we could consider a 'rebase merge'. On a second thought, why does > > > > > > GitHub provide the 'Squash' option? > > > > > > > > > > > > Nathan: +1, We should guard our master branches from direct > pushes. > > > > > > See > > > > > > > > > > > https://github.com/apache/infrastructure-asfyaml?tab=readme-ov-file#branchpro > > > > > > > > > > > > <--- Quick note from Tomek: Nathan totally true, and all > > discussions > > > > > > must be resolved before merge is possible, not sure why its not > > here, > > > > > > could you please take a look and fix on all repos please? > > > > > > > > > > > > > > > > > > > > > > > > ### 18. PR as small as possible. > > > > > > > > > > > > 1. Pull Requests should be as small as possible and focused on > only > > > > > > one functional change. > > > > > > 2. Different functional changes must be provided in separate Pull > > > > > Requests. > > > > > > 3. PR may contain several commits but every single commit > included > > > > > > must not break break overall build, runtime, and compatibility, > > > > > > especially for other components. > > > > > > 4. PR that breaks build or runtime anyhow is considered a > Breaking > > > > > > Change, is not welcome and requires special considerations (see > > > > > > Breaking Changes rule). > > > > > > 5. PR that introduces a new feature must have Documentation > > included > > > > > > in separate commit. > > > > > > 6. When changes for dedicated function 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 > and > > > > > > this is not a Breaking Change. > > > > > > > > > > > > +1 Tomek / PMC > > > > > > +1 Alin / PMC > > > > > > +1 Tiago / PMC > > > > > > +1 TimK > > > > > > +1 Lup / PMC > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan / PMC > > > > > > +1 Dmitri > > > > > > -1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 12 (6 binding), 0:0, -1:1. > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Filipe: +1, item 5 clashes with voting item 8 discussion. > > > > > > > > > > > > Sebastien: +1, I'm ok with a single PR that contains separate > > commits > > > > > > for code and doc. > > > > > > > > > > > > Matteo: -1, #5 PR with new features should not require docs in a > > > > > > separate commit. It's difficult to make changes based on review > and > > > > > > still keep the two commits (code and docs) separate when > squashing. > > > > > > One commit for both should be fine. > > > > > > > > > > > > <--- Quick note from Tomek: Filipe / Matteo I think you talk > about > > > two > > > > > > PR not commits? We will update documentation requirement to be > > along > > > > > > the code in a single PR but separate commits (like it is right > > now). > > > > > > > > > > > > > > > > > > > > > > > > ### 19. Lazy Consensus. > > > > > > > > > > > > A PR may be *eligible* to be merged under the concept of *Lazy > > > > > > consensus* with the following conditions: > > > > > > 1. It affects only a single chip or board (no > > kernel/libs/upper-half > > > > > > drivers etc). > > > > > > 2. It implements a new feature (or app) that doesn't introduce > any > > > > > > breaking changes or backward incompatibility. > > > > > > 3. It didn't get the minimum reviewers after two weeks. > > > > > > 4. At least one independent reviewer reviewed it. > > > > > > 5. It adheres to all other Contributing Guide requirements > > > conditions. > > > > > > > > > > > > The PR's author should: > > > > > > 1. After a week without any reviewers, send an e-mail to the > > mailing > > > > > > list asking for more people to review it. > > > > > > 2. Explain why the PR can't be split into smaller PRs (if > > > applicable). > > > > > > 3. After two weeks ask for the independent reviewer to merge if > > there > > > > > > are no other reviews. The independent reviewer is responsible for > > > > > > checking if the PR matches the *Lazy Consensus* conditions before > > > > > > merging it. > > > > > > > > > > > > -1 Tomek / PMC > > > > > > -1 Alin / PMC > > > > > > -1 Tiago / PMC > > > > > > 0 TimK > > > > > > 0 Lup / PMC > > > > > > 0 Filipe > > > > > > -1 Sebastien > > > > > > -1 Nathan / PMC > > > > > > 0 Dmitri > > > > > > -1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: REJECTED / +1: 3 (1 binding), 0: 4 (1 binding), -1: 6 (4 > > > > > binding). > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Tomek: -1, Considering we are leaving 2 reviewers as is > (increased > > to > > > > > > 4 for breaking changes), lazy consensus may undermine quality, I > > > think > > > > > > this point is not required anymore :-) > > > > > > > > > > > > Alin: -1, We risk critical bugs or harmful code to slip as lazy > > > > > > consensus. > > > > > > > > > > > > Tiago: -1, For the sake of simplicity, let's adopt rule 14 only > and > > > > > > re-evaluate in the future. > > > > > > > > > > > > Lup: 0, I don't think this is priority right now? We can tweak > the > > > > > > guideline later. > > > > > > > > > > > > Sebastien: -1, NERVER EVER let untrusted and unverified code in > > > > > > without a review even if the process is slowed down. If > developer > > > > > > WANTS his code merged the burden is on them to get other > reviewers > > > > > > involved so merge becomes possible. > > > > > > > > > > > > Nathan: -1, I think I suggested Lazy Consensus initially, but > after > > > > > > reading the discussions and comments about this, I think it is > > better > > > > > > to let PR authors to request reviews and keep their PR alive. > > > > > > > > > > > > Alan: +1, Why to increase from 1 week to 2 weeks? > > > > > > > > > > > > > > > > > > -- > > > > > > CeDeROM, SQ7MHZ, http://www.tomek.cedro.info > > > > > > > > > > > > > > > > > > > > -- > > > > > CeDeROM, SQ7MHZ, http://www.tomek.cedro.info > > > > > > > > > > On Sat, Mar 1, 2025 at 7:23 AM Tomek CEDRO <to...@cedro.info> > wrote: > > > > > > > > > > > > Allright Ladies and Gentlemen, here goes the results :-) > > > > > > > > > > > > ACCEPTED: > > > > > > 1. Contributing Guidelines with hints for Reviewers. > > > > > > 2. PR and GIT COMMITS must adhere to Contributing Guidelines or > > > rejected. > > > > > > 3. Git commit messages as important as PR description. > > > > > > 4. Proper description details requirements. > > > > > > 5. PR must adhere to description requirements. > > > > > > 6. Git commit message must adhere to description requirements. > > > > > > 7. Git commit message mandatory fields (topic, desctiption, > > > signature). > > > > > > 9. Zero trust approach to user testing. > > > > > > 11. Respect long term maintenance and self-compatibility. > > > > > > 13. Breaking changes build and runtime test logs are mandatory. > > > > > > 14. Minimum code reviews. > > > > > > 17. Merge rules. > > > > > > 18. PR as small as possible. > > > > > > > > > > > > REJECTED: > > > > > > 8. Changes must come with documentation. > > > > > > 10. Breaking changes not welcome. > > > > > > 16. Self company commit/review/merge not allowed > > > > > > 19. Lazy Consensus. > > > > > > > > > > > > INCOMPLETE: > > > > > > 12. Breaking changes handling process. > > > > > > 15. Reviews independence. > > > > > > > > > > > > > > > > > > Please verify the answers, it took me ~3h of manual responses > > > processing > > > > > ;-) > > > > > > > > > > > > I will next prepare a PR draft with updated guidelines for final > > > > > > polishing and review. In addition to points Accepted I will also > > add > > > > > > points Rejected/Incomplete marked for discussion and hopefully we > > > will > > > > > > reach the agreement on independence, breaking changes, quality, > > > trust, > > > > > > compatibility, and maintenance. > > > > > > > > > > > > Thanks! :-) > > > > > > Tomek > > > > > > > > > > > > > > > > > > ### 1. Contributing Guidelines with hints for Reviewers. > > > > > > > > > > > > We are adding additional section for Reviewers to Contributing > > > > > > Guidelines in order to provide checklist and complementary set of > > > > > > rules that should filter out breaking code as much as possible > also > > > on > > > > > > our side. > > > > > > > > > > > > +1 Tomek / PMC > > > > > > +1 Alin / PMC > > > > > > +1 Tiago / PMC > > > > > > +1 TimK > > > > > > +1 Lup / PMC > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan / PMC > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 13 (6 binding), 0: 0, -1: 0. > > > > > > > > > > > > Remarks: > > > > > > TimH: +1 to all - on the basis that if this doesn't work out > quite > > > > > > right it will be reviewed and changed. Best to try something like > > > this > > > > > > than not - it will become clear very soon if something isn't > > working > > > > > > as intended! > > > > > > > > > > > > > > > > > > > > > > > > ### 2. PR and GIT COMMITS must adhere to Contributing Guidelines > or > > > > > rejected. > > > > > > > > > > > > Each PR and GIT COMMIT **must** adhere to requirements presented > in > > > > > > Contributing Guidelines or will be auto-rejected until fixed / > > > > > > updated. Both code authors and reviewers/committers must follow > the > > > > > > rules. Special cases are defined in a separate dedicated rules. > > > > > > > > > > > > +1 Tomek > > > > > > +1 Alin > > > > > > +1 Tiago > > > > > > +1 TimK > > > > > > +1 Lup > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 13 (6 binding), 0: 0, -1: 0. > > > > > > > > > > > > Remarks: none. > > > > > > > > > > > > > > > > > > > > > > > > ### 3. Git commit messages as important as PR description. > > > > > > > > > > > > Git commit messages are as important as PR descriptions. These > > > provide > > > > > > in-code descriptions of each change and are git interface > > > independent. > > > > > > > > > > > > +1 Tomek > > > > > > +1 Alin > > > > > > +1 Tiago > > > > > > +1 TimK > > > > > > +1 Lup > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 13 (6 binding), 0: 0, -1: 0. > > > > > > > > > > > > Remarks: none. > > > > > > > > > > > > > > > > > > > > > > > > ### 4. Proper description details requirements. > > > > > > > > > > > > Proper description of change is mandatory. Description must > contain > > > > > > explanation on what proposed change do, why it is necessary, what > > if > > > > > > fixes, and how things are changed / fixed / updated, what is the > > > > > > impact (build / runtime / api / what area), how it was tested. > > Local > > > > > > code build and real world hardware runtime test logs must be > > provided > > > > > > (for code related changes). Description can be single..several > > > > > > sentences long or bullet points but enough for anyone to > understand > > > > > > change goals and details. Usually it will look similar for PR and > > git > > > > > > commit message. > > > > > > > > > > > > +1 Tomek > > > > > > +1 Alin > > > > > > +1 Tiago > > > > > > +1 TimK > > > > > > +1 Lup > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 13 (6 binding), 0: 0, -1: 0. > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Tomek: +1, However I understand this PR template is separate from > > the > > > > > > rule and will be updated / voted independently. > > > > > > > > > > > > TimK: +1, Better to have the 'Motivation/Background' section, > while > > > > > > simplifying the rest. In my view, commit messages should address > > the > > > > > > 'What', whereas PR documents should elaborate more on the 'Why'. > > > > > > > > > > > > > > > > > > > > > > > > ### 5. PR must adhere to description requirements. > > > > > > > > > > > > Proper description in PR according to template is mandatory, fill > > in > > > > > > all required fields or change is auto-rejected until fixed / > > > updated. > > > > > > For code changes build and runtime logs are mandatory to prove > code > > > > > > was tested on at least one real world hardware. > > > > > > > > > > > > +1 Tomek > > > > > > +1 Alin > > > > > > +1 Tiago > > > > > > +1 TimK > > > > > > +1 Lup > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 13 (6 binding), 0: 0, -1: 0. > > > > > > > > > > > > Remarks: This point was mostly missing in the answers but no 0 or > > -1 > > > > > > reported so assuming +1, as voted before, and the template > example > > is > > > > > > taken from existing Guidelines, will discuss the PR. > > > > > > > > > > > > > > > > > > PR TEMPLATE / EXAMPLE: > > > > > > > > > > > > Summary > > > > > > 1. Why change is necessary (fix, update, new feature)? > > > > > > 2. What functional part of the code is being changed? > > > > > > 3. How does the change exactly work (what will change and how)? > > > > > > 4. Related NuttX Issue reference if applicable. > > > > > > 5. Related NuttX Apps Issue / Pull Request reference if > applicable. > > > > > > 6. Related NuttX Documentation Pull Request reference if > > applicable. > > > > > > > > > > > > Impact > > > > > > 1. New feature added? Existing feature changed? > > > > > > 2. User (will user need to adapt to change)? NO / YES (please > > > describe > > > > > if yes). > > > > > > 3. Build (will build process change)? NO / YES (please descibe if > > > yes). > > > > > > 4. Hardware (will arch(s) / board(s) / driver(s) change)? NO / > YES > > > > > > (please describe if yes). > > > > > > 5. Documentation (is update required / provided)? NO / YES > (please > > > > > > describe if yes). > > > > > > 6. Security (any sort of implications)? NO / YES (please describe > > if > > > > > yes). > > > > > > 7. Compatibility (backward/forward/interoperability)? NO / YES > > > (please > > > > > > describe if yes). > > > > > > 8. Anything else to consider? > > > > > > > > > > > > Testing > > > > > > > > > > > > 1. I confirm that changes are verified on local setup and works > as > > > > > > intended NO / YES. > > > > > > 2. Build Host(s): OS (Linux,BSD,macOS,Windows,..), > > > CPU(Intel,AMD,ARM), > > > > > > compiler(GCC,CLANG,version), etc. > > > > > > Target(s): arch(sim,RISC-V,ARM,..), board:config, etc. > > > > > > 3. Testing logs before change: > > > > > > runtime / build logs before change goes here > > > > > > 4.Testing logs after change: > > > > > > runtime / build logs after change goes here > > > > > > 5. (optional) How to repeat. You can also provide steps on how to > > > > > > reproduce problem and verify the change if not obvious from test > > > logs. > > > > > > > > > > > > Optional PR remarks: > > > > > > 1. This PR introduces only one functional change. > > > > > > 2. I have updated all required description fields above. > > > > > > 3. My PR adheres to Contributing Guidelines and Documentation > (git > > > > > > commit message, coding standard, testing etc). > > > > > > 4. My PR is still work in progress (not ready for review). > > > > > > 5. My PR is ready for review and can be safely merged into a > > > codebase. > > > > > > > > > > > > > > > > > > > > > > > > ### 6. Git commit message must adhere to description > requirements. > > > > > > > > > > > > Proper GIT COMMIT message according to template is mandatory, or > > > > > > change is rejected until fixed / updates. Build and runtime logs > > are > > > > > > optional here if these are too long and already provided in PR. > > > > > > > > > > > > Git commit message consists of: > > > > > > 1. Topic with functional name prefix, ":" mark, and short > > > > > > self-explanatory context. > > > > > > 2. Blank line > > > > > > 3. Description on what is changed, how, and why. May use several > > > > > > lines, short sentences, or bullet points. > > > > > > 4. Blank line. > > > > > > 5. Signature (created with `git commit -s`). > > > > > > > > > > > > GIT COMMIT TEMPLATE / EXAMPLE: > > > > > > > > > > > > net/can: Add g_ prefix to can_dlc_to_len and len_to_can_dlc. > > > > > > > > > > > > Add g_ prefix to can_dlc_to_len and len_to_can_dlc to > > > > > > follow NuttX coding style conventions for global symbols, > > > > > > improving code readability and maintainability. > > > > > > * you can also use bullet points. > > > > > > * to note different thing briefly. > > > > > > > > > > > > +1 Tomek > > > > > > +1 Alin > > > > > > +1 Tiago > > > > > > +1 TimK > > > > > > +1 Lup > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 13 (6 binding), 0: 0, -1: 0. > > > > > > > > > > > > Remarks: > > > > > > > > > > > > TimK: +1, Regarding the commit message header, I recommend using > > the > > > > > > style adopted by the Angular Community, which is widely accepted. > > > > > > <type>(<scope>): <short summary> > > > > > > > > > > > > Filipe: +1, Let's make sure we have example for this on docs and > > also > > > > > > on PR bot). > > > > > > > > > > > > > > > > > > > > > > > > ### 7. Git commit message mandatory fields (topic, desctiption, > > > > > signature). > > > > > > > > > > > > Each git commit message must consist of topic, description, and > > > > > > signature (git commit -s), as presented in GIT COMMIT TEMPLATE, > > which > > > > > > are mandatory, or change is auto-rejected until fixed / updated. > > > > > > > > > > > > +1 Tomek > > > > > > +1 Alin > > > > > > +1 Tiago > > > > > > +1 TimK > > > > > > +1 Lup > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 13 (6 binding), 0: 0, -1: 0. > > > > > > > > > > > > Remarks: none. > > > > > > > > > > > > > > > > > > > > > > > > ### 8. Changes must come with documentation. > > > > > > > > > > > > Changes must come with with documentation update where > applicable. > > > For > > > > > > maintenance reasons code and documentation should be split into > two > > > > > > separate PR with the same name marked [1/2 CODE] for code and > [2/2 > > > > > > DOC] for documentation. If change presents new functionality a > > > > > > documentation must be provided along with the code (not in > future). > > > If > > > > > > change requires documentation update it must be contained along > > with > > > > > > the code (not in future). Successful documentation build log > > shortcut > > > > > > is welcome. > > > > > > > > > > > > See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > > > > > > > 0: Tomek / PMC > > > > > > 0 Alin / PMC > > > > > > 0 Tiago / PMC > > > > > > -1 TimK > > > > > > 0 Lup / PMC > > > > > > -1 Filipe > > > > > > -1 Sebastien > > > > > > -1 Nathan / PMC > > > > > > 0 Dmitri > > > > > > -1 Matteo > > > > > > 0 Roberto > > > > > > +1 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: REJECTED / +1: 2 (1 binding), 0: 6 (4 binding), -1: 5 (1 > > > > > binding). > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Tomek: 0, Having documentation in a single PR (same pr separate > > > commit > > > > > > is easier to perform and review, otherwise we may get out of > > code/doc > > > > > > sync? But if this is the only way and better for release manager > > then > > > > > > okay. > > > > > > > > > > > > Alin: 0, Having documentation in a single PR (same pr separate > > commit > > > > > > is easier to perform and review. The release process can use it. > > > > > > > > > > > > Tiago: 0, Yes, documentation should be provided, but I don't see > > any > > > > > > reason for splitting it into two different PRs. We keep our > > > > > > documentation in the same repository and - for the sake of > > > > > > traceability - it should be updated in the same PR (separate > > commit, > > > > > > not PR). We should make reviewers' and committers lives easier. > > > > > > Alternative writing would be: > > > > > > "Changes must come with a documentation update where applicable. > > For > > > > > > maintenance reasons, code and documentation should be split into > > two > > > > > > commits in the same PR. If change presents new functionality > > > > > > documentation must be provided along with the code (not in the > > > > > > future). If change requires a documentation update it must be > > > > > > contained along with the code (not in the future)." > > > > > > > > > > > > TimK: -1, I'd like say "should" instead of "must". > > > > > > > > > > > > Lup: 0, It depends? Smaller PRs can include a Doc Commit. When I > > add > > > a > > > > > > new Arch + Board (e.g. StarPro64), the PR will include a link to > my > > > > > > article that explains the new code. Then I prepare another PR for > > the > > > > > > User Docs. > > > > > > > > > > > > Filipe: -1, Don't see any problem in having documentation on same > > PR, > > > > > > in fact I think it makes things easier. > > > > > > > > > > > > Sebastien: -1, separate commits in same pr. > > > > > > > > > > > > Nathan: -1, : Similar to others' comments. Also, could be same PR > > but > > > > > > separate commits? > > > > > > > > > > > > Matteo: -1, : Documentation should be required, in the same PR as > > the > > > > > > change. No separate PRs. > > > > > > > > > > > > > > > > > > PROPOSED UPDATED TEXT: > > > > > > > > > > > > Changes must come with a documentation update where applicable. > For > > > > > > maintenance reasons, code and documentation should be split into > > two > > > > > > commits in the same PR. If change presents new functionality > > > > > > documentation must be provided along with the code (not in the > > > > > > future). If change requires a documentation update it must be > > > > > > contained along with the code (not in the future). Successful > > > > > > documentation build log shortcut is welcome. > > > > > > > > > > > > See: > > > > > > 1. https://github.com/apache/nuttx/tree/master/Documentation > > > > > > 2. https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > > > > > > > > > > > > > <--- Quick note from Tomek: TimK the word "must" is used on > purpose > > > to > > > > > > improve documentation that is usually skipped, and complemented > > > "where > > > > > > applicable" :-) > > > > > > > > > > > > > > > > > > > > > > > > ### 9. Zero trust approach to user testing. > > > > > > > > > > > > We implement zero trust approach to user provided testing. It is > > the > > > > > > commit author duty to provide real world hardware build and > runtime > > > > > > logs for at least one device. Remember that any code change may > > break > > > > > > things for others, please avoid that. > > > > > > > > > > > > See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > > > > > > > +1 Tomek / PMC > > > > > > +1 Alin / PMC > > > > > > +1 Tiago / PMC > > > > > > +1 TimK > > > > > > +1 Lup / PMC > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan / PMC > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > 0 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 12 (5 binding), 0: 1 (1 binding), -1: 0. > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Alan: 0, We cannot transfer this responsibility to the developer, > > he > > > > > > should test it in the HW he has, but we need to have better HW > > > > > > coverage to avoid issues. > > > > > > > > > > > > > > > > > > > > > > > > ### 10. Breaking changes not welcome. > > > > > > > > > > > > Breaking changes are not welcome. We do not "break by design". > When > > > > > > unavoidable breaking changes need prior discussion and agreement > of > > > > > > the community (see Breaking Changes handling rule). This is > > anything > > > > > > that alters Build / Kernel / Architecture / API, alters both > nuttx > > > and > > > > > > nuttx-apps repo at the same time, breaks build/runtime/api for > > single > > > > > > or many boards/architectures/applications, breaks > > self-compatibility, > > > > > > breaks build/runtime compatibility with existing release code > > > > > > (packages) both for nuttx and nuttx-apps, etc. Because thousands > > of > > > > > > users / companies and their projects / products depend on NuttX > > code, > > > > > > we strongly prefer self-compatibility and long-term maintenance > > over > > > > > > "change is good" ideologies. Any code change may impact other > users > > > > > > and their business, please keep that in mind. > > > > > > > > > > > > See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > > > > > > > +1 Tomek / PMC > > > > > > +1 Alin / PMC > > > > > > +1 Tiago / PMC > > > > > > +1 TimK > > > > > > +1 Lup / PMC > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan / PMC > > > > > > +1 Dmitri > > > > > > 0 Matteo > > > > > > +1 Roberto > > > > > > -1 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: REJECTED / +1: 11 (5 binding), 0: 1 , -1: 1 (1 binding). > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Alan: -1, Breaking are necessary sometimes, saying "Breaking > > changes > > > > > > are not welcome" will make people afraid of contributing > innovation > > > > > > that breaks existing APIs. > > > > > > > > > > > > <--- Quick note from Tomek: Alan please re-read that point, it > was > > > > > > updated not to scare people except people that want to break the > > code > > > > > > on purpose, a dedicated section is mentioned and created for > > breaking > > > > > > changes after last round. The whole point of this voting is to > > avoid > > > > > > breaking changes that impact self-compatibility and long term > > > > > > maintenance. It is again blocked without preferred text > > alternative. > > > > > > You gave -1 answer then 0 and asked to use the first form, > correct? > > > > > > > > > > > > > > > > > > > > > > > > ### 11. Respect long term maintenance and self-compatibility > > > > > > > > > > > > We respect long term maintenance and self-compatibility is our > > > > > > ultimate goal. Alternative solutions and non-invasive approaches > > are > > > > > > preferred that offers user a choice and compatibility. Breaking > > > > > > changes are avoided, and planned towards next major release, see > > > > > > Breaking Changes rule. > > > > > > Experimental code that does not impact overall project > > > > > > self-compatibility in terms of Breaking Changes should be clearly > > > > > > marked [EXPERIMENTAL]. > > > > > > > > > > > > See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > > > > > > > > > > > > > +1 Tomek / PMC > > > > > > +1 Alin / PMC > > > > > > +1 Tiago / PMC > > > > > > +1 TimK > > > > > > +1 Lup / PMC > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan / PMC > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > 0 Roberto > > > > > > 0 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 11 (5 binding), 0: 2 (1 binding), -1: 0. > > > > > > > > > > > > Remarks: none. > > > > > > > > > > > > > > > > > > > > > > > > ### 12. Breaking changes handling process. > > > > > > > > > > > > This rule complements "Breaking changes not welcome" rules. We > > avoid > > > > > > breaking changes unless absolutely necessary and unavoidable > (i.e. > > > > > > security fix, broken code, etc), then special case considerations > > may > > > > > > apply: > > > > > > 1. First reviewer that recognizes a breaking change should block > > > > > > accidental merge with "Request Changes" mark and ask for > > discussion. > > > > > > 2. PR is marked as "Draft" to avoid accidental merge. > > > > > > 3. Detailed discussion should follow both in PR AND dev@ Mailing > > > List. > > > > > > 4. Alternative non-breaking alternative solution is researched > with > > > > > > help of the community. > > > > > > 5. Breaking change after discussion / updates is voted on the > > mailing > > > > > > list, requires at least 4 +1 binding votes and single -1 binding > > vote > > > > > > blocks the change (binding vote means PMC member). > > > > > > 6. Breaking changes **must** be verified on various different > real > > > > > > world hardware architectures, build and runtime logs are > > > > > > **mandatory**, help of the community is desired. > > > > > > 7. Breaking change requires at least 4 independent organizations > > > > > > positive PR reviews. > > > > > > 8. Change must be well documented (buid/runtime test logs, pr, > git > > > > > > commit, documentation, release notes, etc). > > > > > > 9. Change must be clearly marked with [BREAKING] mark (pr, git > > > commit, > > > > > > release notes, etc). > > > > > > > > > > > > See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > > > > > > > +1 Tomek / PMC > > > > > > +1 Alin / PMC > > > > > > +1 Tiago / PMC > > > > > > +1 Lup / PMC > > > > > > .. <- votes lost here > > > > > > 0 Roberto > > > > > > -1 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: REJECTED / INCOMPLETE / +1: 5 (4 binding), 0: 1, -1: (1 > > > binding). > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Alan: -1, Mostly a repetition of 10 mixed with 9 and 11. > > > > > > > > > > > > <--- Quick note from Tomek: Alan, this procedure is dedicated for > > > > > > handling breaking changes. If rejecting please provide > alternative > > > > > > expected text. > > > > > > > > > > > > > > > > > > > > > > > > ### 13. Breaking changes build and runtime test logs are > mandatory. > > > > > > > > > > > > Breaking changes are special case where build and runtime test > logs > > > > > > (i.e. apps/ostest) from more than one different architecture is > > > > > > **mandatory** . QEmu tests does not count here as it passed > > breaking > > > > > > change that did not work on a real hardware. Community support is > > > > > > desired. > > > > > > > > > > > > See: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md > > > > > > > > > > > > +1 Tomek > > > > > > +1 Alin > > > > > > +1 Tiago > > > > > > +1 TimK > > > > > > +1 Lup > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > +1 Nathan > > > > > > +1 Dmitri > > > > > > +1 Matteo > > > > > > 0 Roberto > > > > > > 0 Alan > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 11 (5 binding), 0: 2 (1 binding), -1: 0. > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Sebastien: +1, Also some mandatory documentation on how to fix > the > > > > > > build after the breaking change. rust has cargo fix. Python has > > 2to3. > > > > > > > > > > > > Alan: 0, All the changes need to be equally coveraged, not only > > those > > > > > > that break some existing code > > > > > > > > > > > > > > > > > > > > > > > > ### 14. Minimum code reviews. > > > > > > > > > > > > Each PR requires at least 2 independent positive reviews, except > > > > > > Breaking Changes where at least 4 positive independent > > organizations > > > > > > reviews, are required before merge to the upstream. > > > > > > > > > > > > +1 Tomek / PMC > > > > > > +1 Alin / PMC > > > > > > +1 Tiago / PMC > > > > > > -1 TimK > > > > > > +1 Lup / PMC > > > > > > +1 Filipe > > > > > > +1 Sebastien > > > > > > 0 Nathan / PMC > > > > > > 0 Dmitri > > > > > > +1 Matteo > > > > > > +1 Roberto > > > > > > +1 Alan / PMC > > > > > > +1 TimH > > > > > > > > > > > > Result: PASSED / +1: 10 (5 binding), 0: 2 (1 binding), -1: 1. > > > > > > > > > > > > Remarks: > > > > > > > > > > > > Tomek: +1, Although I think 3 should be default to increase > > > cross-checks. > > > > > > > > > > > > Alin: +1, minimum 3. > > > > > > > > > > > > Tiago: +1, I still prefer Nathan's proposal of creating "areas". > > > > > > Documentation and experimental features shouldn't require 2 > > > reviewers. > > > > > > For the sake of simplicity, this rule works. Even 2 reviewers for > > > > > > documentation and experimental features are too restrictive. > > > > > > > > > > > > TimK: -1, "at least 2 independent positive reviews", may be too > > high > > > bar > > > > > we set. > > > > > > > > > > > > Nathan: 0, Tiago summarized my proposal above. However, I am OK > > with > > > > > > whatever the community decides on this. > > > > > > > > > > > > Matteo: +1, Agree that pure documentation changes can have 1 > > > reviewer. > > > > > > > > > > >
