I am sorry for the confusion Please fill in your answers to this email followed by the voter's name to avoid future confusion
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 +1 Alin 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 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 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 ( However I understand this PR template is separate from the rule and will be updated / voted independently.) +1 Alin 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 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 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 (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.) 0 Alin (Having documentation in a single PR (same pr separate commit) is easier to perform and review. The release process can use it) 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 +1 Alin 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 +1 Alin 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 +1 Alin 12. Breaking changes handling process. This rule complements "Breaking changes not welcome" rule. 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 +1 Alin 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 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( Although I think 3 should be default to increase cross-checks.) +1 Alin (minimum 3) 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 +1 Alin 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 +1 Alin 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 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 +1 Alin 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 (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 :-)) -1 Alin (we risk critical bugs or harmful code to slip as lazy consensus )
