From 7d3b5193723ef1928a9108739b0193eecd962f22 Mon Sep 17 00:00:00 2001 From: Chris Kay Date: Mon, 15 Nov 2021 10:50:30 +0000 Subject: [PATCH] docs(commit-style): add commit style documentation This change adds a new documentation page describing the commit style, acceptable Conventional Commits types and scopes, and documents the process for expanding the list of scopes. Change-Id: Iad957b67fa71a879e8aa0790c58a5b08cec300d6 Signed-off-by: Chris Kay --- docs/process/commit-style.rst | 163 ++++++++++++++++++++++++++++++++++ docs/process/contributing.rst | 53 +---------- docs/process/index.rst | 1 + 3 files changed, 166 insertions(+), 51 deletions(-) create mode 100644 docs/process/commit-style.rst diff --git a/docs/process/commit-style.rst b/docs/process/commit-style.rst new file mode 100644 index 000000000..e9df5cee5 --- /dev/null +++ b/docs/process/commit-style.rst @@ -0,0 +1,163 @@ +Commit Style +============ + +When writing commit messages, please think carefully about the purpose and scope +of the change you are making: describe briefly what the change does, and +describe in detail why it does it. This helps to ensure that changes to the +code-base are transparent and approachable to reviewers, and it allows us to +keep a more accurate changelog. You may use Markdown in commit messages. + +A good commit message provides all the background information needed for +reviewers to understand the intent and rationale of the patch. This information +is also useful for future reference. + +For example: + +- What does the patch do? +- What motivated it? +- What impact does it have? +- How was it tested? +- Have alternatives been considered? Why did you choose this approach over + another one? +- If it fixes an `issue`_, include a reference. + +|TF-A| follows the `Conventional Commits`_ specification. All commits to the +main repository are expected to adhere to these guidelines, so it is +**strongly** recommended that you read at least the `quick summary`_ of the +specification. + +To briefly summarize, commit messages are expected to be of the form: + +.. code:: + + [optional scope]: + + [optional body] + + [optional footer(s)] + +The following example commit message demonstrates the use of the +``refactor`` type and the ``amu`` scope: + +.. code:: + + refactor(amu): factor out register accesses + + This change introduces a small set of register getters and setters to + avoid having to repeatedly mask and shift in complex code. + + Change-Id: Ia372f60c5efb924cd6eeceb75112e635ad13d942 + Signed-off-by: Chris Kay + +The following `types` are permissible and are strictly enforced: + ++--------------+---------------------------------------------------------------+ +| Scope | Description | ++==============+===============================================================+ +| ``feat`` | A new feature | ++--------------+---------------------------------------------------------------+ +| ``fix`` | A bug fix | ++--------------+---------------------------------------------------------------+ +| ``build`` | Changes that affect the build system or external dependencies | ++--------------+---------------------------------------------------------------+ +| ``ci`` | Changes to our CI configuration files and scripts | ++--------------+---------------------------------------------------------------+ +| ``docs`` | Documentation-only changes | ++--------------+---------------------------------------------------------------+ +| ``perf`` | A code change that improves performance | ++--------------+---------------------------------------------------------------+ +| ``refactor`` | A code change that neither fixes a bug nor adds a feature | ++--------------+---------------------------------------------------------------+ +| ``revert`` | Changes that revert a previous change | ++--------------+---------------------------------------------------------------+ +| ``style`` | Changes that do not affect the meaning of the code | +| | (white-space, formatting, missing semi-colons, etc.) | ++--------------+---------------------------------------------------------------+ +| ``test`` | Adding missing tests or correcting existing tests | ++--------------+---------------------------------------------------------------+ +| ``chore`` | Any other change | ++--------------+---------------------------------------------------------------+ + +The permissible `scopes` are more flexible, and we maintain a list of them in +our :download:`Commitizen configuration file <../../.cz.json>`. Scopes in this +file are organized by their changelog section, each of which may have one or +more accepted scopes, but only the first of which is considered to be "blessed". +Scopes that are not blessed exist for changes submitted before scope enforcement +came into effect, and are considered deprecated. + +While we don't enforce scopes strictly, we do ask that commits use these if they +can, or add their own if no appropriate one exists (see :ref:`Adding Scopes`). + +It's highly recommended that you use the tooling installed by the optional steps +in the :ref:`prerequisites ` guide to validate commit messages +locally, as commitlint reports a live list of the acceptable scopes. + +.. _Adding Scopes: + +Adding Scopes +------------- + +Scopes that are either a) unblessed in the configuration file, or b) do not +exist in the configuration file at all are considered to be deprecated. If you +are adding a new component that does not yet have a designated scope, please +feel free to add one. + +For example, if you are adding or making modifications to `Foo`'s latest and +greatest new platform `Bar`, you would add it to the `Platforms` changelog +section, and the hierarchy should look something like this: + +.. code:: json + + { + "sections": [ + { + "title": "Platforms", + "sections": [ + { + "title": "Foo", + "scopes": ["foo"], + "sections": [ + { + "title": "Bar", + "scopes": ["bar"] + } + ] + } + ] + } + ] + } + +When creating new scopes, try to keep them short and succinct, and use kebab +case (``this-is-kebab-case``). Components with a product name (i.e. most +platforms and some drivers) should use that name (e.g. ``gic600ae``, +``flexspi``, ``stpmic1``), otherwise use a name that uniquely represents the +component (e.g. ``marvell-comphy-3700``, ``rcar3-drivers``, ``a3720-uart``). + +Mandated Trailers +----------------- + +Commits are expected to be signed off with the ``Signed-off-by:`` trailer using +your real name and email address. You can do this automatically by committing +with Git's ``-s`` flag. + +There may be multiple ``Signed-off-by:`` lines depending on the history of the +patch, but one **must** be the committer. More details may be found in the +`Gerrit Signed-off-by Lines guidelines`_. + +Ensure that each commit also has a unique ``Change-Id:`` line. If you have +followed optional steps in the prerequisites to either install the Node.js tools +or clone the repository using the "`Clone with commit-msg hook`" clone method, +then this should be done automatically for you. + +More details may be found in the `Gerrit Change-Ids documentation`_. + +-------------- + +*Copyright (c) 2021, Arm Limited and Contributors. All rights reserved.* + +.. _Conventional Commits: https://www.conventionalcommits.org/en/v1.0.0 +.. _Gerrit Change-Ids documentation: https://review.trustedfirmware.org/Documentation/user-changeid.html +.. _Gerrit Signed-off-by Lines guidelines: https://review.trustedfirmware.org/Documentation/user-signedoffby.html +.. _issue: https://developer.trustedfirmware.org/project/board/1/ +.. _quick summary: https://www.conventionalcommits.org/en/v1.0.0/#summary diff --git a/docs/process/contributing.rst b/docs/process/contributing.rst index aa050dabe..d6f61d6c7 100644 --- a/docs/process/contributing.rst +++ b/docs/process/contributing.rst @@ -26,23 +26,11 @@ Getting Started Making Changes -------------- +- Ensure commits adhere to the the project's :ref:`Commit Style`. + - Make commits of logical units. See these general `Git guidelines`_ for contributing to a project. -- Ensure your commit messages comply with the `Conventional Commits`_ - specification: - - .. code:: - - [optional scope]: - - [optional body] - - [optional footer(s)] - - You can use the tooling installed by the optional steps in the - :ref:`prerequisites ` guide to validate this locally. - - Keep the commits on topic. If you need to fix another bug or make another enhancement, please address it on a separate topic branch. @@ -52,39 +40,6 @@ Making Changes - Avoid long commit series. If you do have a long series, consider whether some commits should be squashed together or addressed in a separate topic. -- Ensure that each commit in the series has at least one ``Signed-off-by:`` - line, using your real name and email address. The names in the - ``Signed-off-by:`` and ``Commit:`` lines must match. By adding this line the - contributor certifies the contribution is made under the terms of the - :download:`Developer Certificate of Origin <../../dco.txt>`. - - There might be multiple ``Signed-off-by:`` lines, depending on the history - of the patch. - - More details may be found in the `Gerrit Signed-off-by Lines guidelines`_. - -- Ensure that each commit also has a unique ``Change-Id:`` line. If you have - cloned the repository with the "`Clone with commit-msg hook`" clone method - (following the :ref:`Prerequisites` document), this should already be the - case. - - More details may be found in the `Gerrit Change-Ids documentation`_. - -- Write informative and comprehensive commit messages. A good commit message - provides all the background information needed for reviewers to understand - the intent and rationale of the patch. This information is also useful for - future reference. - - For example: - - - What does the patch do? - - What motivated it? - - What impact does it have? - - How was it tested? - - Have alternatives been considered? Why did you choose this approach over - another one? - - If it fixes an `issue`_, include a reference. - - Follow the :ref:`Coding Style` and :ref:`Coding Guidelines`. - Use the checkpatch.pl script provided with the Linux source tree. A @@ -289,15 +244,11 @@ Binary Components *Copyright (c) 2013-2021, Arm Limited and Contributors. All rights reserved.* -.. _Conventional Commits: https://www.conventionalcommits.org/en/v1.0.0 .. _developer.trustedfirmware.org: https://developer.trustedfirmware.org .. _review.trustedfirmware.org: https://review.trustedfirmware.org -.. _issue: https://developer.trustedfirmware.org/project/board/1/ .. _Trusted Firmware-A: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git .. _Git guidelines: http://git-scm.com/book/ch5-2.html .. _Gerrit Uploading Changes documentation: https://review.trustedfirmware.org/Documentation/user-upload.html -.. _Gerrit Signed-off-by Lines guidelines: https://review.trustedfirmware.org/Documentation/user-signedoffby.html -.. _Gerrit Change-Ids documentation: https://review.trustedfirmware.org/Documentation/user-changeid.html .. _TF-A Tests: https://trustedfirmware-a-tests.readthedocs.io .. _Trusted Firmware binary repository: https://review.trustedfirmware.org/admin/repos/tf-binaries .. _tf-binaries-readme: https://git.trustedfirmware.org/tf-binaries.git/tree/readme.rst diff --git a/docs/process/index.rst b/docs/process/index.rst index 37324b0e9..bba2b40eb 100644 --- a/docs/process/index.rst +++ b/docs/process/index.rst @@ -8,6 +8,7 @@ Processes & Policies security platform-compatibility-policy + commit-style coding-style coding-guidelines contributing