Hi, 1: +1 2: +1 3: +1 4: +1 5: +1 6: +1 7: +1
8: +1 I heavily recommend sending the documentation at the same PR. Considering that PRs are as atomic as possible, the documentation should be tightly linked with it. It would not create merge issues (at least not an additional workload) and having them on separate PRs may lead to errors if left behind when backporting. 9: +1 10: *-1* Breaking changes are not welcomed, but sometimes they are necessary. I think the next item (11) already covers the care we need with breaking changes. 11: +1 12: +1 13: +1 14: +1 15: +1 16: +1 17: +1 Em ter., 11 de fev. de 2025 às 07:06, Ville Juven <ville.ju...@unikie.com.invalid> escreveu: > 1-4: +1 > 5: 0 > > * > Description for PR and commit are a given, how is anyone expected to > understand what you are trying to do (and why), without a description? That > is what the commit text field is for.. > * > However, demanding build and run-time logs from downstream targets is not > feasible and I seldom test on targets that are upstream, with the exception > of simulator targets. Many times severe issues in the kernel do not reveal > themselves in simulated environments, so testing in those is close to > meaningless; if it works in simulation, it does not mean it works. But then > again, if it FAILS in simulation, it means it doesn't work for sure. This > is why I vote 0. Use discretion here. The bigger the change, the more you / > we must demand. > > 6:+1 > 7: +1 > > * > Though, why is the signature significant? I never understood this point. > I'm not arguing against making it mandatory, I'm just genuinely curious. > > 8: +1, as long as a bullet point list "this is how you build&run this > target"-type of documentation is enough. > 9: +1, How about we don't break things for others, period ? This is the > biggest issue IMO, changes are made to targets that don't need to be > changed (Sebastien's issues are of this type). Platforms that are not being > used/maintained by the person/organization pushing for the change should > NEVER be touched. > 10: +1 Absolutely, at least without a consensus that the change makes > things clearly better. Not different, BETTER. > 11-16: +1 > 17: +1 and if this ever happens it should be severely punished. > > -Ville Juven > > ________________________________ > From: Tomek CEDRO <to...@cedro.info> > Sent: Tuesday, February 11, 2025 1:37 AM > To: dev@nuttx.apache.org <dev@nuttx.apache.org> > Subject: [VOTE] NuttX Contributing Guidelines update 202502. > > 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, > https://eur05.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.tomek.cedro.info%2F&data=05%7C02%7Cville.juven%40unikie.com%7Cf17c9957b3b8435e55c208dd4a2bf02b%7C9207ba3377314e38abf1c7183f53f8c3%7C0%7C0%7C638748274753974831%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=Y8Q2yUb1wWB2xP4blZdT9JDrNecffIMPU%2Bo2DiEKzik%3D&reserved=0 > <http://www.tomek.cedro.info/> >
