doc: contribution guidelines: Clarify and extend

Clarify and extend some of the PR and contribution guidelines so that
they cover practices that have been effectively enforced by maintainers,
but were never properly documented.

Signed-off-by: Carles Cufi <[email protected]>
Signed-off-by: Anas Nashif <[email protected]>

Author: carlescufi

doc/build/kconfig/tips.rst

@@ -876,31 +876,10 @@ For a Kconfig symbol that enables a driver/subsystem FOO, consider having just
 usually be clear in the context of an option that can be toggled on/off, and
 makes things consistent.
 
+Style
+=====
 
-Header comments and other nits
-==============================
-
-A few formatting nits, to help keep things consistent:
-
-- Use this format for any header comments at the top of ``Kconfig`` files:
-
-  .. code-block:: none
-
-     # <Overview of symbols defined in the file, preferably in plain English>
-     (Blank line)
-     # Copyright (c) 2019 ...
-     # SPDX-License-Identifier: <License>
-     (Blank line)
-     (Kconfig definitions)
-
-- Format comments as ``# Comment`` rather than ``#Comment``
-
-- Put a blank line before/after each top-level ``if`` and ``endif``
-
-- Use a single tab for each indentation
-
-- Indent help text with two extra spaces
-
+See :ref:`coding_style` for style guidelines.
 
 Lesser-known/used Kconfig features
 **********************************

doc/contribute/contributor_expectations.rst

@@ -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.
 
+- 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:
+
+     * 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
+  introduced by the PR. The documentation changes must use the proper
+  terminology as present in the existing pages, and must be written in American
+  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.
 

doc/contribute/documentation/guidelines.rst

@@ -700,6 +700,8 @@ Cross-referencing C documentation
 Visual Elements
 ***************
 
+.. _doc_images:
+
 Images
 ======
 

doc/contribute/guidelines.rst

@@ -536,12 +536,28 @@ reference manuals, etc.
 Coding Style
 ============
 
+.. note::
+   Coding style is enforced on any new or modified code, but contributors are
+   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.
 * The line length is 100 columns or fewer. In the documentation, longer lines
   for URL references are an allowed exception.
 * Add braces to every ``if``, ``else``, ``do``, ``while``, ``for`` and
@@ -553,6 +569,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:
+
+* CMake
+
+  * Indent with spaces, indentation is two spaces.
+  * Don't use space between commands (e.g. ``if``) and the corresponding opening
+    bracket (e.g. ``(``).
+
+* Devicetree
+
+  * 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.
+  * 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.
+
+* Kconfig
+
+  * Line length of 100 columns or fewer.
+  * 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.

doc/project/project_roles.rst

@@ -350,6 +350,7 @@ Release Activity
 Merge Criteria
 ++++++++++++++
 
+* All :ref:`pr_requirements` must be met.
 * Minimal of 2 approvals, including an approval by the designated assignee.
 * Pull requests should be reviewed by at least a maintainer or collaborator of
   each affected area; Unless the changes to a given area are considered trivial