doc/contribute/contributor_expectations.rst
.. _contributor-expectations:
Contributor Expectations ########################
The Zephyr project encourages :ref:contributors <contributor> to submit
changes as smaller pull requests. Smaller pull requests (PRs) have the following
benefits:
Reviewed more quickly and reviewed more thoroughly. It's easier for reviewers to set aside a few minutes to review smaller changes several times than it is to allocate large blocks of time to review a large PR.
Less wasted work if reviewers or maintainers reject the direction of the change.
Easier to rebase and merge. Smaller PRs are less likely to conflict with other changes in the tree.
Easier to revert if the PR breaks functionality.
.. note:: This page does not apply to draft PRs which can have any size, any number of commits and any combination of smaller PRs for testing and preview purposes. Draft PRs have no review expectation and PRs created as drafts from the start do not notify anyone by default.
Defining Smaller PRs
Smaller PRs should encompass one self-contained logical change.
When adding a new large feature or API, the PR should address only one part of
the feature. In this case create an :ref:RFC proposal <rfcs> to describe the
additional parts of the feature for reviewers.
PRs should include tests or samples under the following conditions:
Adding new features or functionality.
Modifying a feature, especially for API behavior contract changes.
Fixing a hardware agnostic bug. The test should fail without the bug fixed and pass with the fix applied.
PRs must update any documentation affected by a functional code change.
If introducing a new API, the PR must include an example usage of the API. This provides context to the reviewer and prevents submitting PRs with unused APIs.
Multiple Commits on a Single PR
Contributors are further encouraged to break up PRs into multiple commits. Keep in mind each commit in the PR must still build cleanly and pass all the CI tests.
For example, when introducing an extension to an API, contributors can break up the PR into multiple commits targeting these specific changes:
#. Introduce the new APIs, including shared devicetree bindings #. Update driver implementation X, with driver specific devicetree bindings #. Update driver implementation Y #. Add tests for the new API #. Add a sample using the API #. Update the documentation
Large Changes
Large changes to the Zephyr project must submit an :ref:RFC proposal <rfcs>
describing the full scope of change and future work. The RFC proposal provides
the required context to reviewers, but allows for smaller, incremental, PRs to
get reviewed and merged into the project. The RFC should also define the minimum
viable implementation.
Changes which require an RFC proposal include:
treewide-changes.Maintainers have the discretion to request that contributors create an RFC for PRs that are too large or complicated.
.. _pr_requirements:
PR Requirements
.. important::
Pull requests that do not meet the quality expectations outlined below are less likely to receive reviewer engagement, and reviewers may request changes on such pull requests without detailed feedback.
Contributors are expected to self-review their changes and ensure all requirements are met before requesting a review.
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>.
The code must follow the Zephyr :ref:coding_style and :ref:coding_guidelines.
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.
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:
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.
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.
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:
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:
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:
Attribution of Authorship
While cleaning up the commit history, ensure that authorship attribution remains accurate.
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 and reviewers have the discretion to determine if the provided tests are sufficient. The examples below demonstrate best practices on how to test APIs effectively.
Kernel timer tests <tests/kernel/timer/timer_behavior>
provide around 85% test coverage for the
:zephyr_file:kernel timer <kernel/timer.c>, measured by lines of code.fuel gauge tests <tests/drivers/fuel_gauge/sbs_gauge>
use the :zephyr_file:smart battery emulator <drivers/fuel_gauge/sbs_gauge/emul_sbs_gauge.c>,
providing test coverage for the
:zephyr_file:fuel gauge API <include/zephyr/drivers/fuel_gauge.h>
and the :zephyr_file:smart battery driver <drivers/fuel_gauge/sbs_gauge/sbs_gauge.c>.Codecov_.Incompatible changes to APIs must also update the release notes for the next release detailing the change. APIs marked as experimental are excluded from this requirement.
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.
Maintainers may request that contributors break up a PR into smaller PRs and may
request that they create an :ref:RFC proposal <rfcs>.
.. _Codecov: https://app.codecov.io/gh/zephyrproject-rtos/zephyr
Unless they applied the reviewer's recommendation exactly, authors must not resolve and hide comments, they must let the initial reviewer do it. The Zephyr project does not require all comments to be resolved before merge. Leaving some completed discussions open can sometimes be useful to understand the greater picture.
Respond to comments using the "Start Review" and "Add Review" green buttons in the "Files changed" view. This allows responding to multiple comments and publishing the responses in bulk. This reduces the number of emails sent to reviewers.
As GitHub does not implement |git range-diff|_, try to minimize rebases in the middle of a review. If a rebase is required, push this as a separate update with no other changes since the last push of the PR. When pushing a rebase only, add a comment to the PR indicating which commit is the rebase.
.. |git range-diff| replace:: git range-diff
.. _git range-diff: https://git-scm.com/docs/git-range-diff
The Zephyr community is a diverse group of individuals, with different levels of commitment and priorities. As such, reviewers and maintainers may not get to a PR right away.
After 1 week of inactivity, ping the Assignee or reviewers on the PR by adding a comment to the PR.
After 2 weeks of inactivity, post a message on the #pr-help_ channel on
Discord linking to the PR.
.. _pr_technical_escalation:
In cases where a contributor objects to change requests from reviewers, Zephyr defines the following escalation process for resolving technical disagreements.
Before escalation of technical disagreements, follow the steps below:
Resolve in the PR among assignee, maintainers and reviewer.
If no progress is made, the assignee (maintainer) has the right to dismiss stale, unrelated or irrelevant change requests by reviewers giving the reviewers a minimum of 1 business day to respond and revisit their initial change requests or start the escalation process.
The assignee has the responsibility to document the reasoning for dismissing any reviews in the PR and should notify the reviewer about their review being dismissed.
To give the reviewers time to respond and escalate, the assignee is expected to block the PR from being merged either by not approving the PR or by setting the DNM label.
Escalation can be triggered by any party participating in the review process (assignee, reviewers or the original author of the change) following the steps below:
Escalate to the Architecture Working Group_ by adding the Architecture Review label on the PR. Beside the weekly meeting where such escalations are
processed, the Architecture Working Group_ shall facilitate an offline
review of the escalation if requested, especially if any of the parties can't
attend the meeting.
If all avenues of resolution and escalation have failed, assignees can escalate to the TSC and get a binding resolution in the TSC by adding the TSC label on the PR.
The Assignee is expected to ensure the resolution of the escalation and the outcome is documented in the related pull request or issues on Github.
.. _#pr-help: https://discord.com/channels/720317445772017664/997527108844798012
.. _Architecture Project: https://github.com/zephyrproject-rtos/zephyr/projects/18
.. _Architecture Working Group: https://github.com/zephyrproject-rtos/zephyr/wiki/Architecture-Working-Group