-
Notifications
You must be signed in to change notification settings - Fork 7.6k
doc: contribution guidelines: Clarify and extend #83117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,27 +87,83 @@ Changes which require an RFC proposal include: | |
Maintainers have the discretion to request that contributors create an RFC for | ||
PRs that are too large or complicated. | ||
|
||
.. _pr_requirements: | ||
|
||
PR Requirements | ||
*************** | ||
|
||
- Each commit in the PR must provide a commit message following the | ||
:ref:`commit-guidelines`. | ||
|
||
- No fixup or merge commits are allowed, see :ref:`Contribution workflow` for | ||
more information. | ||
|
||
- The PR description must include a summary of the changes and their rationales. | ||
|
||
- All files in the PR must comply with :ref:`Licensing | ||
Requirements<licensing_requirements>`. | ||
|
||
- Follow the Zephyr :ref:`coding_style` and :ref:`coding_guidelines`. | ||
- The code must follow the Zephyr :ref:`coding_style` and :ref:`coding_guidelines`. | ||
|
||
- PRs must pass all CI checks. This is a requirement to merge the PR. | ||
- The PR must pass all CI checks, as described in :ref:`merge_criteria`. | ||
Contributors may mark a PR as draft and explicitly request reviewers to | ||
provide early feedback, even with failing CI checks. | ||
|
||
- When breaking a PR into multiple commits, each commit must build cleanly. The | ||
- When breaking up a PR into multiple commits, each commit must build cleanly. The | ||
CI system does not enforce this policy, so it is the PR author's | ||
responsibility to verify. | ||
Comment on lines
+112
to
114
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking - but this paragraph is now duplicated by the "Retain Bisectability" below. |
||
|
||
- Commits in a pull request should represent clear, logical units of change that are easy to review | ||
and maintain bisectability. The following guidelines expand on this principle: | ||
|
||
1. Distinct, Logical Units of Change | ||
|
||
Each commit should correspond to a self-contained, meaningful change. For example, adding a | ||
feature, fixing a bug, or refactoring existing code should be separate commits. Avoid mixing | ||
different types of changes (e.g., feature implementation and unrelated refactoring) in the same | ||
commit. | ||
|
||
2. Retain Bisectability | ||
|
||
Every commit in the pull request must build successfully and pass all relevant tests. This | ||
ensures that git bisect can be used effectively to identify the specific commit that introduced | ||
a bug or issue. | ||
|
||
3. Squash Intermediary or Non-Final Development History | ||
|
||
During development, commits may include intermediary changes (e.g., partial implementations, | ||
temporary files, or debugging code). These should be squashed or rewritten before submitting the | ||
pull request. Remove non-final artifacts, such as: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As partial implementations, ... should be squashed before submitting a zephyr PR, doesn't this exclude the possibility to cooperate on a PR ? Or does this mean that at every stage of development on a zephyr PR the commits should be squashed before submitting the update ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can collaborate in a PR and mark it as a draft, when ready for review, the commits shall follow the guidelines. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the initial draft becomes too messy after some time, you can also close it and submit other, cleaner PR(s) to make the final review easier and faster. This is relatively common. Don't forget to link back to the initial one in case some reviewers have time for the long and messy story. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that what could be considered "partial implementation" are fixup type commits. While having some work presented in stages, for not only in draf, the commits should have rather reviewable quality, and should be squashed to one before merge. For example if you move something around file, or files, and then change the logic, it may aid reviewers to pick these changes in separate commits to see how the move happened, and then diff of the moved code. Having these separate on review makes sense, but on tree does not as the file is tested in CI as this was single-commit change, and probably neither "partial" commit can be removed without voiding that. On the other hand fixups do not help, because you when you pick one of commits in PR and then there is fixup for some logic, then you have a problem reviewing either commit: one is broken and second is incomplete without the first one, and does not give you full view of a change anyway. Let me throw here recent PR here #84058, where you can see a potential for two commit, for review time, change, where large block of text is remove and changed at the same time ;). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @de-nordic renaming and changing at the same time is a common enough use case, not always well handled by git, so maybe it is missing here. I agree it's MUCH easier to review when separate! Both before and AFTER merge. On the other hand, this PR is clearly insisting on reviewability and there are already recommendations here and elsewhere to have "atomic" commits. So hopefully the risk of misinterpretation is low? To avoid stalling this PR again, how about a follow-up one from you? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Small follow-up submitted: |
||
|
||
* Temporary renaming of files that are later renamed again. | ||
* Code that was rewritten or significantly changed in later commits. | ||
|
||
4. Ensure Clean History Before Submission | ||
|
||
Use interactive rebasing (git rebase -i) to clean up the commit history before submitting the | ||
pull request. This helps in: | ||
|
||
* Squashing small, incomplete commits into a single cohesive commit. | ||
* Ensuring that each commit remains bisectable. | ||
* Maintaining proper attribution of authorship while improving clarity. | ||
|
||
5. Renaming and Code Rewrites | ||
|
||
If files or code are renamed or rewritten in later commits during development, squash or rewrite | ||
earlier commits to reflect the final structure. This ensures that: | ||
|
||
* The history remains clean and easy to follow. | ||
* Bisectability is preserved by eliminating redundant renaming or partial rewrites. | ||
|
||
6. Attribution of Authorship | ||
|
||
While cleaning up the commit history, ensure that authorship attribution remains accurate. | ||
|
||
7. Readable and Reviewable History | ||
|
||
The final commit history should be easy to understand for future maintainers. Logical units of | ||
change should be grouped into commits that tell a clear, coherent story of the work done. | ||
|
||
- When major new functionality is added, tests for the new functionality shall | ||
be added to the automated test suite. All API functions should have test cases | ||
and there should be tests for the behavior contracts of the API. Maintainers | ||
|
@@ -133,6 +189,13 @@ PR Requirements | |
- Changes to APIs must increment the API version number according to the API | ||
version rules. | ||
|
||
- Documentation must be added and/or updated to reflect the changes in the code | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is already covered and is a duplication, see https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#defining-smaller-prs. Maybe it is important enough that it should be mentioned twice, my worry is that our guidelines and docs are growing organically and in a negative way, i.e. you might be adding something here that conflicts with somewhere else mentioned in some other part of the docs. |
||
introduced by the PR. The documentation changes must use the proper | ||
terminology as present in the existing pages, and must be written in American | ||
carlescufi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
English. If you include images as part of the documentation, those must follow | ||
the rules in :ref:`doc_images`. Please refer to :ref:`doc_guidelines` for | ||
additional information. | ||
|
||
- PRs must also satisfy all :ref:`merge_criteria` before a member of the release | ||
engineering team merges the PR into the zephyr tree. | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -700,6 +700,8 @@ Cross-referencing C documentation | |
Visual Elements | ||
*************** | ||
|
||
.. _doc_images: | ||
|
||
Images | ||
====== | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -536,12 +536,28 @@ reference manuals, etc. | |||||||||||||||||||||||||||||||||||||||||||||
Coding Style | ||||||||||||||||||||||||||||||||||||||||||||||
============ | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
.. note:: | ||||||||||||||||||||||||||||||||||||||||||||||
nordicjm marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
Coding style is enforced on any new or modified code, but contributors are | ||||||||||||||||||||||||||||||||||||||||||||||
carlescufi marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
not expected to correct the style on existing code that they are not | ||||||||||||||||||||||||||||||||||||||||||||||
modifying. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
.. note:: | ||||||||||||||||||||||||||||||||||||||||||||||
For style aspects where the guidelines don't offer explicit guidance or | ||||||||||||||||||||||||||||||||||||||||||||||
permit multiple valid ways to express something, contributors should follow | ||||||||||||||||||||||||||||||||||||||||||||||
the style of existing code in the tree, with higher importance given to | ||||||||||||||||||||||||||||||||||||||||||||||
"nearby" code (first look at the function, then the same file, then | ||||||||||||||||||||||||||||||||||||||||||||||
subsystem, etc). | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
.. _Linux kernel coding style: | ||||||||||||||||||||||||||||||||||||||||||||||
https://kernel.org/doc/html/latest/process/coding-style.html | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
.. _snake case: | ||||||||||||||||||||||||||||||||||||||||||||||
https://en.wikipedia.org/wiki/Snake_case | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
In general, follow the `Linux kernel coding style`_, with the following | ||||||||||||||||||||||||||||||||||||||||||||||
exceptions: | ||||||||||||||||||||||||||||||||||||||||||||||
exceptions and clarifications: | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
* Use `snake case`_ for code and variables. | ||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but this is covered by linux already, in details, I do not think we need to repeat this here: https://kernel.org/doc/html/latest/process/coding-style.html#naming There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not explicit some said, so it was made explicit here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm in favor of incremental additions to our style guide that eventually make reference the Linux style guide obsolete. While unlikely, Linux can change their style guidelines at any time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. need to just fork the original linux style into our docs, call them zephyr style guidelines and never look back again. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
the more I look at this, the more I think we should just go our own and stop referencing this from Linux. It looks more of a rant than guidelines in some parts. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, totally, and I wanted to point this out the other day. It seriously feels very dated in the way it's written, to the point that it's not even reflecting the values we try to promote via our Code of Conduct, for example... Like, doing something wrong being refered to as being a "shooting offense"... Really?! So yeah if there's something where we can clearly state "we're not Linux" maybe it can start with coding style :) big +1 from me There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Those are very good arguments. I've never before read the Linux coding guidelines like that, but you are absolutely right. |
||||||||||||||||||||||||||||||||||||||||||||||
* The line length is 100 columns or fewer. In the documentation, longer lines | ||||||||||||||||||||||||||||||||||||||||||||||
for URL references are an allowed exception. | ||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
561
to
562
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given there are quite many places where line length is over 100 columns, does the rule "try to align with style in nearby code" apply? I.e. we're now saying we're not expecting contributors to fix non-conforming code/files, so do we want them to introduce potentially oddly/inconsistently wrapped content to existing files? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Up to others to comment on this, but for me this question really falls on the "case by case" and "common sense" criteria of the reviewers. I don't think rewriting the text or complicating it further will help solve all the corner cases like this one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New code should comply with the requirements There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In principle I do not think this text needs changing, but ping me if you disagree @kartben. |
||||||||||||||||||||||||||||||||||||||||||||||
* Add braces to every ``if``, ``else``, ``do``, ``while``, ``for`` and | ||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -554,6 +570,38 @@ exceptions: | |||||||||||||||||||||||||||||||||||||||||||||
* Avoid using binary literals (constants starting with ``0b``). | ||||||||||||||||||||||||||||||||||||||||||||||
* Avoid using non-ASCII symbols in code, unless it significantly improves | ||||||||||||||||||||||||||||||||||||||||||||||
clarity, avoid emojis in any case. | ||||||||||||||||||||||||||||||||||||||||||||||
* Use proper capitalization of nouns in code comments (e.g. ``UART`` and not | ||||||||||||||||||||||||||||||||||||||||||||||
``uart``, ``CMake`` and not ``cmake``). | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
Beyond C code, the following coding style rules apply to other types of files: | ||||||||||||||||||||||||||||||||||||||||||||||
fabiobaltieri marked this conversation as resolved.
Show resolved
Hide resolved
carlescufi marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
* CMake | ||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agree with the couple of guidelines listed here, however, i think this is just touching the surface and we should be adding guidelines for each area and try to be complete and comprehensive and not just list those that someone happened to flag in some recent PR. if we want to have some rules and guidelines for cmake files, we should spend the time to do this proper and call this done, if not, one second after this PR is merged we might be dealing with something new that would block another PR for ages, i.e. someone might not like the use for I propose to split this out and submit a PR for adding cmake style guidelines that goes beyond the above, ideally follow some work done in the cmake community itself and adopt such rules or use some large projects depending on cmake (Qt, KDE, ...) and borrow the rules from there where it makes sense. Do that once, make sure the rules are as complete as possible, get this in and we do not have to come back to this topic again, unless there is something new and significant. To support the above, here is a draft. #83713 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the draft PR and for the proposal. If we go this way though, we should probably then open two more PRs, one for Devicetree and one for Kconfig. And we should also decide whether we want the coding style to be in a single place (as it is in this PR) or in each respective section (as you propose with your draft CMake PR). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I like having one location for coding style - a single location provides is easier to find for new contributors (and for seasoned contributors to reference in PR comments). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can always cross reference them and still list them in one place. i.e. we reference linux styles and do not have them directly in the same page, we can do this with others. As each style guide for each components grows, it will all be one page and become unreadable/maintainable. |
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
* Indent with spaces, indentation is two spaces. | ||||||||||||||||||||||||||||||||||||||||||||||
* Don't use space between commands (e.g. ``if``) and the corresponding opening | ||||||||||||||||||||||||||||||||||||||||||||||
bracket (e.g. ``(``). | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
* Devicetree | ||||||||||||||||||||||||||||||||||||||||||||||
carlescufi marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
* Indent with tabs. | ||||||||||||||||||||||||||||||||||||||||||||||
* Follow the Devicetree specification conventions and rules. | ||||||||||||||||||||||||||||||||||||||||||||||
* Use dashes (``-``) as word separators for node and property names. | ||||||||||||||||||||||||||||||||||||||||||||||
* Use underscores (``_``) as word separators in node labels. | ||||||||||||||||||||||||||||||||||||||||||||||
carlescufi marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
* Leave a single space on each side of the equal sign (``=``) in property | ||||||||||||||||||||||||||||||||||||||||||||||
definitions. | ||||||||||||||||||||||||||||||||||||||||||||||
* Don't insert empty lines before a dedenting ``};``. | ||||||||||||||||||||||||||||||||||||||||||||||
* Insert a single empty line to separate nodes at the same hierarchy level. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seemingly we need something to prevent random newline spacing in the same parts i.e. https://github.com/zephyrproject-rtos/zephyr/pull/83310/files#diff-a188c48a9493b631f516e6672a23b4792710f6c3e7375e7b4956957a9c22d3edR18 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will leave this for a a second pass at it, don't want to extend the scope right now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a clear example of being too pick. Having a new line on that part makes easy to ready IMHO. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The files in that PR are a complete mess, compare that with other board files, it's completely at odds with that and doesn't even use the same layout in the same file, changing multiple times. It does not look easy to read There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From your pr:
So to answer:
We need it so people i.e. like you in your PR don't come up with your own styles completely at odds with what is standard in zephyr, we don't need every board or every subsystem to have their own coding styles which add nothing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can either of you provide some stats perhaps? if most of the current code complies with one approach or other perhaps we can include it in the list. Otherwise I would rather not tread there to avoid overloading this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I understand it, @nandojve does not want us to introduce guidelines that discourage empty lines between properties within a node. I agree. Grouping properties using newlines is common practice in DTS, both in Zephyr and elsewhere. I don't currently see anything in the content of this PR for introducing such guidelines, so perhaps we can leave it out of this PR - and address it separately, if anybody desires? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Out of 886 board dts files in zephyr, the following have spaces in aliases:
4 out of 886 = 0.4% There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you put your script here? It seems to be too selective. zephyr/boards/atmel/sam/sam4l_ek/sam4l_ek.dts Lines 52 to 58 in dabeb9c
zephyr/boards/atmel/sam/sam_e70_xplained/sam_e70_xplained-common.dtsi Lines 57 to 62 in dabeb9c
Is this change only valid for the boards directory? Or we are looking only there because @nordicjm is a collaborator of that part. BTW who is 57300 ? Lines 594 to 602 in dabeb9c
Nevermind, GH really allow a number be person. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As said, only for aliases, https://pastebin.com/tuv8VLsK
It's there because that's where board aliases are |
||||||||||||||||||||||||||||||||||||||||||||||
* Kconfig | ||||||||||||||||||||||||||||||||||||||||||||||
carlescufi marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
* Line length of 100 columns or fewer. | ||||||||||||||||||||||||||||||||||||||||||||||
carlescufi marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? This makes sense in the context of Kconfig, because in Devicetree it is not enforced unless I am mistaken? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because I was under the impression that the enclosing section, "Coding style", already applies to everything? Why would this need to be repeated for Kconfig? And why is it not repeated for CMake, then?
Oh is it not? I had no idea fwiw :) and I don't think that's what the text says in its current form either. If there are exceptions they need to be mentioned otherwise it reads as everything is 100 columns or fewer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reading this again it looks like "Coding Style" is probably meant to read "C coding style", and that you probably didn't mean to fold Kconfig/devicetree/CMake "under" this section as this has really nothing to do with C, and can mistakenly lead people like myself to then interpret "Coding guidelines" as being "General style guidelines" (and e.g. interpret the bullet "In the documentation, longer lines for URL references are an allowed exception." as applying to RST files, whereas I guess the intent was to apply to Doxygen documentation?) Related(ish): The "Coding guidelines" section is pretty buried in the document. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What you say is correct and would benefit from a cleanup, but I am trying really really hard to limit the scope of this PR. If I start now rewriting the Coding Style section I will open a can of worms that I would rather leave closed in this iteration, we can then do another pass cleaning up. |
||||||||||||||||||||||||||||||||||||||||||||||
* Indent with tabs, except for ``help`` entry text which should be placed at | ||||||||||||||||||||||||||||||||||||||||||||||
one tab plus two extra spaces. | ||||||||||||||||||||||||||||||||||||||||||||||
* Leave a single empty line between option declarations. | ||||||||||||||||||||||||||||||||||||||||||||||
* Use Statements like ``select`` carefully, see | ||||||||||||||||||||||||||||||||||||||||||||||
:ref:`kconfig_tips_and_tricks` for more information. | ||||||||||||||||||||||||||||||||||||||||||||||
* Format comments as ``# Comment`` rather than ``#Comment`` | ||||||||||||||||||||||||||||||||||||||||||||||
* Insert an empty line before/after each top-level ``if`` and ``endif`` | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
Use these coding guidelines to ensure that your development complies with the | ||||||||||||||||||||||||||||||||||||||||||||||
project's style and naming conventions. | ||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"must follow coding guidelines" contradicts what coding guidelines actually say -- the linked page says we "begin enforcement on a limited [and undocumented?] scope" as we're preparing to enter "Stage II", and stage I literally says that PR cannot be blocked when there are violations.
Talked to @nashif and it might be that this whole stage thingy is just obsolete. cc @keith-zephyr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#84058