adafruit-mirror/zephyr
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 9

doc: improvements to Contribution Guidelines

Browse source 
Restructure the Contribution Guidelines page in the documentation.

Create the 'Pull Request Guidelines' section containing the best
practices for creating Pull Requests.

Improve commit message guidelines: instruct contributors to refer to
GitHub issue in the PR message instead of commit message when
submitting a fix; add a description of `Link:` tags for refering to
other relevant information.

Move 'clang-format' and 'twister' subsections to the new PR
Guidelines section.

Delete outdated/incorrect information about how to run tests locally
with twister.

Describe the `check_compliance.py` script as the recommended tool for
checking compliance of changes and how to run the script locally.
Remove 'gitlint' subsection.

Add a list of recommended steps for getting a new PR accepted quickly.

Signed-off-by: Gustavo Silva <gustavograzs@gmail.com>
This commit is contained in:
Gustavo Silva 2024-01-13 19:36:49 -03:00 committed by Alberto Escolar
parent 7a5f2ad46e
commit 9e2c773137
1 changed files with 288 additions and 249 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

537
doc/contribute/guidelines.rst
View file

@ -148,6 +148,8 @@ For your commits, replace:
You can automatically add the Signed-off-by: line to your commit body using You can automatically add the Signed-off-by: line to your commit body using
``git commit -s``. Use other commits in the zephyr git history as examples. ``git commit -s``. Use other commits in the zephyr git history as examples.
See :ref:`git_setup` for instructions on configuring user and email settings
in Git.
Additional requirements: Additional requirements:
@ -331,16 +333,10 @@ business days.
You can find all `open pull requests`_ on GitHub and open `Zephyr Project You can find all `open pull requests`_ on GitHub and open `Zephyr Project
Issues`_ in Github issues. Issues`_ in Github issues.
.. _Continuous Integration: .. _git_setup:
Git Setup
Tools and Git Setup *********
*******************
.. _git-name-and-email:
Name and email
==============
We need to know who you are, and how to contact you. To add this We need to know who you are, and how to contact you. To add this
information to your Git installation, set the Git configuration information to your Git installation, set the Git configuration
@ -355,93 +351,181 @@ address is ``z.developer@example.com``:
git config --global user.name "Zephyr Developer" git config --global user.name "Zephyr Developer"
git config --global user.email "z.developer@example.com" git config --global user.email "z.developer@example.com"
gitlint
=========
When you submit a pull request to the project, a series of checks are Pull Request Guidelines
performed to verify your commit messages meet the requirements. The same step ***********************
done during the CI process can be performed locally using the ``gitlint`` When opening a new Pull Request, adhere to the following guidelines to ensure
command. compliance with Zephyr standards and facilitate the review process.
Run ``gitlint`` locally in your tree and branch where your patches have been If in doubt, it's advisible to explore existing Pull Requests within the Zephyr
committed: repository. Use the search filters and labels to locate PRs related to changes
similar to the ones you are proposing.
.. code-block:: console .. _commit-guidelines:
gitlint Commit Message Guidelines
=========================
Note, gitlint only checks HEAD (the most recent commit), so you should run it Changes are submitted as Git commits. Each commit has a *commit
after each commit, or use the ``--commits`` option to specify a commit range message* describing the change. Acceptable commit messages look like
covering all the development patches to be submitted. this:
twister .. code-block:: none
=======
.. note:: [area]: [summary of change]
twister support on windows is limited and execution of tests is not
supported, only building.
To verify that your changes did not break any tests or samples, please run the [Commit message body (must be non-empty)]
``twister`` script locally before submitting your pull request to GitHub.
Twister allows limiting the scope of the tests built and run by pointing it to Signed-off-by: [Your Full Name] <[your.email@address]>
the tests related to the code or the platform you have modified. For example, to
limit tests to a single platform and an area in the kernel::
source zephyr-env.sh You need to change text in square brackets (``[like this]``) above to
west twister -p qemu_x86 -T tests/kernel/sched fit your commit.
Running tests on connected devices is also supported using the Examples and more details follow.
``--device-testing`` options. Please consult with the :ref:`Twister
<twister_script>` documentation for more details.
To run the same tests the CI system runs, follow these steps from within your Example
local Zephyr source working directory: -------
.. code-block:: console Here is an example of a good commit message.
source zephyr-env.sh .. code-block:: none
west twister --integration
The above will execute the basic twister script, which will run various drivers: sensor: abcd1234: fix bus I/O error handling
tests using the QEMU emulator and other simulators supported in Zephyr.
It will also do some build tests on various samples with advanced features that
can't run in a simulator or QEMU.
We highly recommend you run these tests locally to avoid any CI failures The abcd1234 sensor driver is failing to check the flags field in
However, note that building and executing tests using twister requires the response packet from the device which signals that an error
significant computing resources. When running locally and to get results in a occurred. This can lead to reading invalid data from the response
reasonable time, limit the scope to the areas and platforms you have modified. buffer. Fix it by checking the flag and adding an error path.
In case of major changes to the kernel, build or configuration infrastructures
of Zephyr, it is advised to use twister for verifying majority the changes
before handing over to the dedicated CI resources provided by the Zephyr
project.
clang-format Signed-off-by: Zephyr Developer <z.developer@example.com>
============
The `clang-format tool <https://clang.llvm.org/docs/ClangFormat.html>`_ can [area]: [summary of change]
be helpful to quickly reformat large amounts of new source code to our ---------------------------
`Coding Style`_ standards together with the ``.clang-format`` configuration file
provided in the repository. ``clang-format`` is well integrated into most
editors, but you can also run it manually like this:
.. code-block:: bash This line is called the commit's *title*. Titles must be:
clang-format -i my_source_file.c * one line
* less than 72 characters long
* followed by a completely blank line
``clang-format`` is part of LLVM, which can be downloaded from the project [area]
`releases page <https://github.com/llvm/llvm-project/releases>`. Note that if The ``[area]`` prefix usually identifies the area of code
you are a Linux user, ``clang-format`` will likely be available as a package in being changed. It can also identify the change's wider
your distribution repositories. context if multiple areas are affected.
Here are some examples:
* ``doc: ...`` for documentation changes
* ``drivers: foo:`` for ``foo`` driver changes
* ``Bluetooth: Shell:`` for changes to the Bluetooth shell
* ``net: ethernet:`` for Ethernet-related networking changes
* ``dts:`` for treewide devicetree changes
* ``style:`` for code style changes
If you're not sure what to use, try running ``git log FILE``, where
``FILE`` is a file you are changing, and using previous commits that
changed the same file as inspiration.
[summary of change]
The ``[summary of change]`` part should be a quick description of
what you've done. Here are some examples:
* ``doc: update wiki references to new site``
* ``drivers: sensor: sensor_shell: fix channel name collision``
Commit Message Body
-------------------
.. warning::
An empty commit message body is not permitted. Even for trivial
changes, please include a descriptive commit message body. Your
pull request will fail CI checks if you do not.
This part of the commit should explain what your change does, and why
it's needed. Be specific. A body that says ``"Fixes stuff"`` will be
rejected. Be sure to include the following as relevant:
* **what** the change does,
* **why** you chose that approach,
* **what** assumptions were made, and
* **how** you know it works -- for example, which tests you ran.
Each line in your commit message should usually be 75 characters or
less. Use newlines to wrap longer lines. Exceptions include lines
with long URLs, email addresses, etc.
For examples of accepted commit messages, you can refer to the Zephyr GitHub
`changelog <https://github.com/zephyrproject-rtos/zephyr/commits/main>`__.
Signed-off-by: ...
------------------
.. tip::
You should have set your :ref:`git_setup`
already. Create your commit with ``git commit -s`` to add the
Signed-off-by: line automatically using this information.
For open source licensing reasons, your commit must include a
Signed-off-by: line that looks like this:
.. code-block:: none
Signed-off-by: [Your Full Name] <[your.email@address]>
For example, if your full name is ``Zephyr Developer`` and your email
address is ``z.developer@example.com``:
.. code-block:: none
Signed-off-by: Zephyr Developer <z.developer@example.com>
This means that you have personally made sure your change complies
with the :ref:`DCO`. For this reason, you must use your legal name.
Pseudonyms or "hacker aliases" are not permitted.
Your name and the email address you use must match the name and email
in the Git commit's ``Author:`` field.
See the :ref:`contributor-expectations` for a more complete discussion of
contributor and reviewer expectations.
Adding links
------------
.. _GitHub references:
https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/autolinked-references-and-urls
Do not include `GitHub references`_ in the commit message directly, as it can
lose meaning in case the repository is forked, for example. Instead, if the
change addresses a specific GitHub issue, include in the Pull Request message a
line of the form:
.. code-block:: none
Fixes #[issue number]
Where ``[issue number]`` is the relevant GitHub issue's number. For
example:
.. code-block:: none
Fixes: #1234
You can point to other relevant information that can be found on the web using
:code:`Link:` tags. This includes, for example: GitHub issues, datasheets,
reference manuals, etc.
.. code-block:: none
Link: https://github.com/zephyrproject-rtos/zephyr/issues/<issue number>
.. _coding_style: .. _coding_style:
Coding Style Coding Style
************ ============
Use these coding guidelines to ensure that your development complies with the
project's style and naming conventions.
.. _Linux kernel coding style: .. _Linux kernel coding style:
https://kernel.org/doc/html/latest/process/coding-style.html https://kernel.org/doc/html/latest/process/coding-style.html
@ -462,8 +546,8 @@ exceptions:
* Avoid using non-ASCII symbols in code, unless it significantly improves * Avoid using non-ASCII symbols in code, unless it significantly improves
clarity, avoid emojis in any case. clarity, avoid emojis in any case.
When there are differences between the guidelines above and the formatting Use these coding guidelines to ensure that your development complies with the
generated by code formatting tools, the guidelines above take precedence. project's style and naming conventions.
The Linux kernel GPL-licensed tool ``checkpatch`` is used to check The Linux kernel GPL-licensed tool ``checkpatch`` is used to check
coding style conformity. coding style conformity.
@ -506,8 +590,102 @@ before pushing on zephyr repo. To do this, make the file
If you want to override checkpatch verdict and push you branch despite reported If you want to override checkpatch verdict and push you branch despite reported
issues, you can add option --no-verify to the git push command. issues, you can add option --no-verify to the git push command.
A more complete alternative to this is using check_compliance.py script from A more complete alternative to this is using :ref:`check_compliance_py` script.
ci-tools repo.
clang-format
------------
The `clang-format tool <https://clang.llvm.org/docs/ClangFormat.html>`_ can
be helpful to quickly reformat large amounts of new source code to our
`Coding Style`_ standards together with the ``.clang-format`` configuration file
provided in the repository. ``clang-format`` is well integrated into most
editors, but you can also run it manually like this:
.. code-block:: bash
clang-format -i my_source_file.c
``clang-format`` is part of LLVM, which can be downloaded from the project
`releases page <https://github.com/llvm/llvm-project/releases>`_. Note that if
you are a Linux user, ``clang-format`` will likely be available as a package in
your distribution repositories.
When there are differences between the `Coding Style`_ guidelines and the
formatting generated by code formatting tools, the `Coding Style`_ guidelines
take precedence. If there is ambiguity between formatting tools and the
guidelines, maintainers may decide which style should be adopted.
.. _Continuous Integration:
Continuous Integration (CI)
===========================
The Zephyr Project operates a Continuous Integration (CI) system that runs on
every Pull Request (PR) in order to verify several aspects of the PR:
* Git commit formatting
* Coding Style
* Twister builds for multiple architectures and boards
* Documentation build to verify any doc changes
CI is run on Github Actions and it uses the same tools described in the
`CI Tests`_ section. The CI results must be green indicating "All
checks have passed" before the Pull Request can be merged. CI is run when the
PR is created, and again every time the PR is modified with a commit.
The current status of the CI run can always be found at the bottom of the
GitHub PR page, below the review status. Depending on the success or failure
of the run you will see:
* "All checks have passed"
* "All checks have failed"
In case of failure you can click on the "Details" link presented below the
failure message in order to navigate to ``Github Actions`` and inspect the
results.
Once you click on the link you will be taken to the ``Github actions`` summary
results page where a table with all the different builds will be shown. To see
what build or test failed click on the row that contains the failed (i.e.
non-green) build.
.. _CI Tests:
Running CI Tests Locally
========================
.. _check_compliance_py:
check_compliance.py
-------------------
The ``check_compliance.py`` script serves as a valuable tool for assessing code
compliance with Zephyr's established guidelines and best practices. The script
acts as wrapper for a suite of tools that performs various checks, including
linters and formatters.
Developers are encouraged to run the script locally to validate their changes
before opening a new Pull Request:
.. code-block:: bash
./scripts/ci/check_compliance.py -c upstream/main..
twister
-------
.. note::
twister is only fully supported on Linux; on Windows and MacOS the execution
of tests is not supported, only building.
If you think your change may break some test, you can submit your PR as a draft
and let the project CI automatically run the :ref:`twister_script` for you.
If a test fails, you can check from the CI run logs how to rerun it locally,
for example:
.. code-block:: bash
west twister -p native_sim -s tests/drivers/build_all/sensor/sensors.generic_test
.. _static_analysis: .. _static_analysis:
@ -560,8 +738,6 @@ it after completing the steps above on scan service website. Any issues
closed without a fix or without ignoring the entry in the scan service will be closed without a fix or without ignoring the entry in the scan service will be
automatically reopened if the issue continues to be present in the code. automatically reopened if the issue continues to be present in the code.
.. _Contribution Tools:
.. _Contribution workflow: .. _Contribution workflow:
Contribution Workflow Contribution Workflow
@ -687,13 +863,7 @@ workflow here:
and use the same process described above to work on this new topic branch. and use the same process described above to work on this new topic branch.
#. If reviewers do request changes to your patch, you can interactively rebase #. If reviewers do request changes to your patch, you can interactively rebase
commit(s) to fix review issues. In your development repo:: commit(s) to fix review issues. In your development repo::
git fetch --all
git rebase --ignore-whitespace upstream/main
The ``--ignore-whitespace`` option stops ``git apply`` (called by rebase)
from changing any whitespace. Continuing::
git rebase -i <offending-commit-id>^ git rebase -i <offending-commit-id>^
@ -715,6 +885,17 @@ workflow here:
By force pushing your update, your original pull request will be updated By force pushing your update, your original pull request will be updated
with your changes so you won't need to resubmit the pull request. with your changes so you won't need to resubmit the pull request.
#. After pushing the requested change, check on the PR page if there is a
merge conflict. If so, rebase your local branch::
git fetch --all
git rebase --ignore-whitespace upstream/main
The ``--ignore-whitespace`` option stops ``git apply`` (called by rebase)
from changing any whitespace. Resolve the conflicts and push again::
git push --force origin fix_comment_typo
.. note:: While amending commits and force pushing is a common review model .. note:: While amending commits and force pushing is a common review model
outside GitHub, and the one recommended by Zephyr, it's not the main outside GitHub, and the one recommended by Zephyr, it's not the main
model supported by GitHub. Forced pushes can cause unexpected behavior, model supported by GitHub. Forced pushes can cause unexpected behavior,
@ -729,151 +910,41 @@ workflow here:
Additional information about the CI system can be found in Additional information about the CI system can be found in
`Continuous Integration`_. `Continuous Integration`_.
.. _commit-guidelines: .. _contribution_tips:
Commit Message Guidelines Contribution Tips
************************* =================
Changes are submitted as Git commits. Each commit has a *commit The following is a list of tips to improve and accelerate the review process of
message* describing the change. Acceptable commit messages look like Pull Requests. If you follow them, chances are your pull request will get the
this: attention needed and it will be ready for merge sooner than later:
.. code-block:: none .. _git-rebase:
https://git-scm.com/docs/git-rebase#Documentation/git-rebase.txt---keep-base
[area]: [summary of change] #. When pushing follow-up changes, use the ``--keep-base`` option of
`git-rebase`_
[Commit message body (must be non-empty)] #. On the PR page, check if the change can still be merged with no merge
conflicts
Signed-off-by: [Your Full Name] <[your.email@address]> #. Make sure title of PR explains what is being fixed or added
You need to change text in square brackets (``[like this]``) above to #. Make sure your PR has a body with more details about the content of your
fit your commit. submission
Examples and more details follow. #. Make sure you reference the issue you are fixing in the body of the PR
Example #. Watch early CI results immediately after submissions and fix issues as they
======= are discovered
Here is an example of a good commit message. #. Revisit PR after 1-2 hours to see the status of all CI checks, make sure all
is green
.. code-block:: none #. If you get request for changes and submit a change to address them, make
sure you click the "Re-request review" button on the GitHub UI to notify
those who asked for the changes
drivers: sensor: abcd1234: fix bus I/O error handling
The abcd1234 sensor driver is failing to check the flags field in
the response packet from the device which signals that an error
occurred. This can lead to reading invalid data from the response
buffer. Fix it by checking the flag and adding an error path.
Signed-off-by: Zephyr Developer <z.developer@example.com>
[area]: [summary of change]
===========================
This line is called the commit's *title*. Titles must be:
* one line
* less than 72 characters long
* followed by a completely blank line
[area]
The ``[area]`` prefix usually identifies the area of code
being changed. It can also identify the change's wider
context if multiple areas are affected.
Here are some examples:
* ``doc: ...`` for documentation changes
* ``drivers: foo:`` for ``foo`` driver changes
* ``Bluetooth: Shell:`` for changes to the Bluetooth shell
* ``net: ethernet:`` for Ethernet-related networking changes
* ``dts:`` for treewide devicetree changes
* ``style:`` for code style changes
If you're not sure what to use, try running ``git log FILE``, where
``FILE`` is a file you are changing, and using previous commits that
changed the same file as inspiration.
[summary of change]
The ``[summary of change]`` part should be a quick description of
what you've done. Here are some examples:
* ``doc: update wiki references to new site``
* ``drivers: sensor: sensor_shell: fix channel name collision``
Commit Message Body
===================
.. warning::
An empty commit message body is not permitted. Even for trivial
changes, please include a descriptive commit message body. Your
pull request will fail CI checks if you do not.
This part of the commit should explain what your change does, and why
it's needed. Be specific. A body that says ``"Fixes stuff"`` will be
rejected. Be sure to include the following as relevant:
* **what** the change does,
* **why** you chose that approach,
* **what** assumptions were made, and
* **how** you know it works -- for example, which tests you ran.
Each line in your commit message should usually be 75 characters or
less. Use newlines to wrap longer lines. Exceptions include lines
with long URLs, email addresses, etc.
For examples of accepted commit messages, you can refer to the Zephyr GitHub
`changelog <https://github.com/zephyrproject-rtos/zephyr/commits/main>`__.
If the change addresses a GitHub issue, include a line of the form:
.. code-block:: none
Fixes #[issue number]
Where ``[issue number]`` is the relevant GitHub issue's number. For
example:
.. code-block:: none
Fixes: #1234
Signed-off-by: ...
==================
.. tip::
You should have set your :ref:`git-name-and-email`
already. Create your commit with ``git commit -s`` to add the
Signed-off-by: line automatically using this information.
For open source licensing reasons, your commit must include a
Signed-off-by: line that looks like this:
.. code-block:: none
Signed-off-by: [Your Full Name] <[your.email@address]>
For example, if your full name is ``Zephyr Developer`` and your email
address is ``z.developer@example.com``:
.. code-block:: none
Signed-off-by: Zephyr Developer <z.developer@example.com>
This means that you have personally made sure your change complies
with the :ref:`DCO`. For this reason, you must use your legal name.
Pseudonyms or "hacker aliases" are not permitted.
Your name and the email address you use must match the name and email
in the Git commit's ``Author:`` field.
Other Commit Expectations
=========================
See the :ref:`contributor-expectations` for a more complete discussion of
contributor and reviewer expectations.
Submitting Proposals Submitting Proposals
==================== ====================
@ -930,38 +1001,6 @@ For example, a copy of an externally maintained import in a module repository::
commit: 08ded7f21529c39e5133688ffb93a9d0c94e5c6e commit: 08ded7f21529c39e5133688ffb93a9d0c94e5c6e
Purpose: Introduction of TinyCrypt Purpose: Introduction of TinyCrypt
Continuous Integration (CI)
***************************
The Zephyr Project operates a Continuous Integration (CI) system that runs on
every Pull Request (PR) in order to verify several aspects of the PR:
* Git commit formatting
* Coding Style
* Twister builds for multiple architectures and boards
* Documentation build to verify any doc changes
CI is run on Github Actions and it uses the same tools described in the
`Contribution Tools`_ section. The CI results must be green indicating "All
checks have passed" before the Pull Request can be merged. CI is run when the
PR is created, and again every time the PR is modified with a commit.
The current status of the CI run can always be found at the bottom of the
GitHub PR page, below the review status. Depending on the success or failure
of the run you will see:
* "All checks have passed"
* "All checks have failed"
In case of failure you can click on the "Details" link presented below the
failure message in order to navigate to ``Github Actions`` and inspect the
results.
Once you click on the link you will be taken to the ``Github actions`` summary
results page where a table with all the different builds will be shown. To see
what build or test failed click on the row that contains the failed (i.e.
non-green) build.
Contributions to External Modules Contributions to External Modules
********************************** **********************************