Skip to content

Commit

Permalink
doc: contribution guidelines: Clarify and extend
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
carlescufi authored and nashif committed Jan 15, 2025
1 parent 0709478 commit 391f82e
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 28 deletions.
27 changes: 3 additions & 24 deletions doc/build/kconfig/tips.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
**********************************
Expand Down
69 changes: 66 additions & 3 deletions doc/contribute/contributor_expectations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.

Expand Down
2 changes: 2 additions & 0 deletions doc/contribute/documentation/guidelines.rst
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,8 @@ Cross-referencing C documentation
Visual Elements
***************

.. _doc_images:

Images
======

Expand Down
50 changes: 49 additions & 1 deletion doc/contribute/guidelines.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:

* 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.
Expand Down
1 change: 1 addition & 0 deletions doc/project/project_roles.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 391f82e

Please sign in to comment.