Alin, but this is not an enforced rule of Linux, see: https://github.com/torvalds/linux/commit/35fa2d88ca9481e5caf533d58b99ca259c63b2fe https://github.com/torvalds/linux/commit/6a774228e890ee04a0ee13f4e6e731ec8554b9c2 https://github.com/torvalds/linux/commit/d0caf9876a1c9f844307effb598ad1312d9e0025 https://github.com/torvalds/linux/commit/170aafe35cb98e0f3fbacb446ea86389fbce22ea https://github.com/torvalds/linux/commit/27069e7cb3d1cea9377069266acf19b9cc5ad0ae https://github.com/torvalds/linux/commit/5f5e7344322f0b0676579af054c787ed57d1c1df
You can search there and find thousands of commits with documentation and code together. BR, Alan On Fri, Mar 7, 2025 at 6:52 AM Alin Jerpelea <jerpe...@gmail.com> wrote: > Hi all, > > I was always looking at the kernel.org is a good development practice > since > it has a huge community and it has been around for some time > > https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/Documentation/bpf?h=v6.14-rc5 > > if we decide that it is too hard to split commits then let's drop the > requirement > > Best regards > Alin > > On Fri, Mar 7, 2025 at 10:43 AM raiden00pl <raiden0...@gmail.com> wrote: > > > 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. > > > > > > > > > > > > > > > > > > > >
