1. +1
2. +1
3. +1
4. +1
5. +1
6. +1
7. +1
8. +1
9. +1
10. -1
Broken API, broken features and any other broken code should be removed
even if
it breaks some users code. Legacy code means more work for maintainers,
worse
code quality and inconvenience for users. It's always been like this in
NuttX,
which is why NuttX's code is readable and easy to follow. Changing this can
be
a huge downside for the project.
11. -1
Not every interface in NuttX is equally mature. Until we establish which API
is stable and which is not, this change is bad. Breaking a stable API should
be avoided, breaking an unstable and under development API should be
allowed.
12. 0
Not all breaking changes are equal.
13. +1
14. -1
Not all PRs are equal.
For important PRs it's OK, but as a requirement for all PRs - nope.
15. 0
Single company review should not be allowed and that's enough.
There are areas where 2 reviews from the same company has more value
than 2 reviews from the independent people with no knowledge about a given
area. The quality of the review is more important than the company it
comes from.
16. +1
17. +1
wt., 11 lut 2025 o 09:18 Alin Jerpelea <jerpe...@gmail.com> napisał(a):
> 1: +1 2: +1 3: +1 4: +1 5: +1 6: +1 7: +1 8: -1 documentation should be
> provided at the same time as a separate PR with the same name and {2/2} to
> indicate that they belong to the same PR. For LTS documentation may cause
> merge issues and increase the maintainers workload 9: +1 10: +1 11: +1 PRs
> should contain only 1 area ex drivers, arch, boards to make LTS port easy.
> PR should use the same title followed by the sequence PR with {number/total
> change PR} :ex arch/arm/cxd56: add feature x {1/2}, boards/arm/cxd56: add
> feature x {2/2} 12: +1 13: +1 14: +1 15: +1 16: +1 17: +1
>
> On Tue, Feb 11, 2025 at 8:41 AM Michal Lenc <michall...@seznam.cz> wrote:
>
> > Hi,
> >
> > 1: +1
> > 2: +1
> > 3: +1
> > 4: +1
> > 5: +1
> > 6: +1
> > 7: +1
> > 8: +1
> > 9: +1
> > 10: 0 these are sometimes necessary
> > 11: +1
> > 12: +1
> > 13: +1
> > 14: -1 I would still apply it only for bigger changes
> > 15: +1
> > 16: +1
> > 17: +1
> >
> > Thanks for organizing the vote!
> >
> > Michal
> >
> > On 2/11/25 00:37, Tomek CEDRO 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
> > >
> >
>
