1. +1 2. +1 3. +1 4. +1 5. +1 6. +1 definitely shouldn't put runtime logs in the commit description 7. +1 8. +1 9. +1 10. -1, breaking changes shouldn't be unwelcome so long as they are discussed an agreed upon by the community via the mailing list first. Some criteria on what is considered sufficient approval should probably be made. 11. +1 12. +1 13. +1 14. +1 (maybe even 3 would be okay though, there seems to be a shortage of reviewers) 15. +1 16. +1 17. +1
On Tue, Feb 11, 2025 at 11:05 AM Alan C. Assis <acas...@gmail.com> wrote: > 1. +1 > 2. +1 > 3. +1 > 4. +1 > 5. +1 > 6. +1 > 7. +1 > 8. +1 > 9. +1 // this is something we need to improve, HW CI should confirm if a > PR is working > 10. -1 > > In some cases breaking previous compatibility is necessary to evolution of > the project, but these breaking need to be discussed further in the > mainling list. > > 11. 0 > > We don't have stable APIs in the kernel yet (only POSIX userspace), if > "long term maintenance" means LTS support and if this impact our few > contributors and reviewers, it needs to be discussed further. Forcing > contributors to create a single PR for each modification in diferent areas > of the kernel doesn't sound feasible. Commits and PR need to have logic > separation, if a Documentation/ is about something that touches the current > PR, it is fine to be in the same commit. > > 12. +1 > > 13. +1 > > 14. -1 > > 15. -1 > > 16. 0 > > 17. -1 > > If it was reviewed by other peers (2 or more) and the PR still there > waiting to be merge for more than 5 days after review, the author (if he > has commit write permission) could have the right to merge it. > > On Mon, Feb 10, 2025 at 8:39 PM Tomek CEDRO <to...@cedro.info> wrote: > > > Hello world :-) > > > > As discussed extensively in various mailing list threads we have > > gathered all additional ideas for Contributing Guidelines update that > > should improve NuttX Code Quality and self-compatibility / long term > > maintenance. > > > > Lets vote what we have. If anything is missing then lets talk about > > this in "NuttX Code Quality Improvement 2025Q1" thread and add to vote > > here the final form. > > > > Each proposal is given number, please vote +1, 0, or -1 in reply for > > every number to vote for, neutral, or against proposed update. > > Comments are welcome too :-) > > > > 1. In Contributing Guidelines we are adding additional section for > > Reviewers in order to provide complementary set of rules that should > > filter out breaking code as much as possible also on our side. > > > > 2. Each PR (including git commits) _must_ adhere to requirements > > presented in Contributing Guidelines or will be auto-rejected until > > fixed / updated. > > > > 3. Git commit messages are as important as PR descriptions. These > > provide in-code descriptions of each change and are git interface > > independent. > > > > 4. 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 > > where mandatory. 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. > > > > 5. Proper description in PR is mandatory, or change is auto-rejected > > until fixed / updated. Build and real world hardware runtime logs are > > mandatory. > > > > 6. Proper description in Git commit message 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. > > > > 7. Each git commit message must consist of topic, description, and > > signature, which are mandatory, or change is auto-rejected until fixed > > / updated. Topic consists of functional prefix, ":" mark, and short > > self-explanatory context. Description is separated from topic with a > > single blank line. Example already presented in Contributing > > Guidelines. > > > > 8. Changes must come with with documentation update where applicable. > > If change presents new functionality a documentation must be provided > > in the same PR (not in future). If change requires documentation > > update it must be contained in the same PR (not in future). Successful > > documentation build log shortcut is welcome. > > > > 9. We implement zero trust approach to user provided testing. It is > > the commit author duty to provide real world hardware build and > > runtime logs that must prove change does not break stuff for others. > > > > 10. Breaking changes are not welcome. 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. > > > > 11. 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. > > > > 12. Breaking changes _must_ be discussed prior introduction on the > > dev@ mailing list. PR may be created with clear indication it is for > > discussion and marked as draft not to be "accidentally merged". > > > > 13. 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. > > > > 14. Number of minimum required code review votes should be increased > > from 2 to 4. This will ensure cross-checks and filter out faulty > > changes. > > > > 15. Counting review votes should come from independent organizations. > > There may be more than one review from a single organization, but > > these will count as one vote. > > > > 16. Single company commit, review, merge is not allowed. > > > > 17. Self committed code merge is not allowed. > > > > Thank you :-) > > Tomek > > > > -- > > CeDeROM, SQ7MHZ, http://www.tomek.cedro.info > > >
