Paul Beesley | fc9ee36 | 2019-03-07 15:47:15 +0000 | [diff] [blame] | 1 | Contributor's Guide |
| 2 | =================== |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 3 | |
| 4 | Getting Started |
| 5 | --------------- |
| 6 | |
Sandrine Bailleux | d314734 | 2020-05-12 10:36:05 +0200 | [diff] [blame] | 7 | - Make sure you have a Github account and you are logged on both |
| 8 | `developer.trustedfirmware.org`_ and `review.trustedfirmware.org`_. |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 9 | |
Sandrine Bailleux | 6ef77ce | 2020-08-03 10:27:19 +0200 | [diff] [blame] | 10 | - If you plan to contribute a major piece of work, it is usually a good idea to |
| 11 | start a discussion around it on the mailing list. This gives everyone |
| 12 | visibility of what is coming up, you might learn that somebody else is |
| 13 | already working on something similar or the community might be able to |
| 14 | provide some early input to help shaping the design of the feature. |
| 15 | |
| 16 | If you intend to include Third Party IP in your contribution, please mention |
| 17 | it explicitly in the email thread and ensure that the changes that include |
| 18 | Third Party IP are made in a separate patch (or patch series). |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 19 | |
Paul Beesley | d2fcc4e | 2019-05-29 13:59:40 +0100 | [diff] [blame] | 20 | - Clone `Trusted Firmware-A`_ on your own machine as described in |
| 21 | :ref:`prerequisites_get_source`. |
Sandrine Bailleux | 75804e5 | 2020-08-12 11:29:46 +0200 | [diff] [blame] | 22 | |
John Tsichritzis | 2fd3d92 | 2019-05-28 13:13:39 +0100 | [diff] [blame] | 23 | - Create a local topic branch based on the `Trusted Firmware-A`_ ``master`` |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 24 | branch. |
| 25 | |
| 26 | Making Changes |
| 27 | -------------- |
| 28 | |
| 29 | - Make commits of logical units. See these general `Git guidelines`_ for |
| 30 | contributing to a project. |
Sandrine Bailleux | 75804e5 | 2020-08-12 11:29:46 +0200 | [diff] [blame] | 31 | |
Chris Kay | 98b26e3 | 2020-12-09 16:52:03 +0000 | [diff] [blame] | 32 | - Ensure your commit messages comply with the `Conventional Commits`_ |
| 33 | specification: |
| 34 | |
| 35 | .. code:: |
| 36 | |
| 37 | <type>[optional scope]: <description> |
| 38 | |
| 39 | [optional body] |
| 40 | |
| 41 | [optional footer(s)] |
| 42 | |
| 43 | You can use the tooling installed by the optional steps in the |
| 44 | :ref:`prerequisites <Prerequisites>` guide to validate this locally. |
| 45 | |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 46 | - Keep the commits on topic. If you need to fix another bug or make another |
Sandrine Bailleux | 6ef77ce | 2020-08-03 10:27:19 +0200 | [diff] [blame] | 47 | enhancement, please address it on a separate topic branch. |
Sandrine Bailleux | 75804e5 | 2020-08-12 11:29:46 +0200 | [diff] [blame] | 48 | |
Sandrine Bailleux | e0cb189 | 2020-08-14 15:58:50 +0200 | [diff] [blame] | 49 | - Split the patch in manageable units. Small patches are usually easier to |
| 50 | review so this will speed up the review process. |
| 51 | |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 52 | - Avoid long commit series. If you do have a long series, consider whether |
| 53 | some commits should be squashed together or addressed in a separate topic. |
Sandrine Bailleux | 75804e5 | 2020-08-12 11:29:46 +0200 | [diff] [blame] | 54 | |
Sandrine Bailleux | e0cb189 | 2020-08-14 15:58:50 +0200 | [diff] [blame] | 55 | - Ensure that each commit in the series has at least one ``Signed-off-by:`` |
| 56 | line, using your real name and email address. The names in the |
| 57 | ``Signed-off-by:`` and ``Commit:`` lines must match. By adding this line the |
| 58 | contributor certifies the contribution is made under the terms of the |
| 59 | :download:`Developer Certificate of Origin <../../dco.txt>`. |
| 60 | |
| 61 | There might be multiple ``Signed-off-by:`` lines, depending on the history |
| 62 | of the patch. |
| 63 | |
| 64 | More details may be found in the `Gerrit Signed-off-by Lines guidelines`_. |
| 65 | |
| 66 | - Ensure that each commit also has a unique ``Change-Id:`` line. If you have |
| 67 | cloned the repository with the "`Clone with commit-msg hook`" clone method |
| 68 | (following the :ref:`Prerequisites` document), this should already be the |
| 69 | case. |
| 70 | |
| 71 | More details may be found in the `Gerrit Change-Ids documentation`_. |
| 72 | |
| 73 | - Write informative and comprehensive commit messages. A good commit message |
| 74 | provides all the background information needed for reviewers to understand |
| 75 | the intent and rationale of the patch. This information is also useful for |
| 76 | future reference. |
| 77 | |
| 78 | For example: |
| 79 | |
| 80 | - What does the patch do? |
| 81 | - What motivated it? |
| 82 | - What impact does it have? |
| 83 | - How was it tested? |
| 84 | - Have alternatives been considered? Why did you choose this approach over |
| 85 | another one? |
| 86 | - If it fixes an `issue`_, include a reference. |
| 87 | |
| 88 | - Follow the :ref:`Coding Style` and :ref:`Coding Guidelines`. |
| 89 | |
| 90 | - Use the checkpatch.pl script provided with the Linux source tree. A |
| 91 | Makefile target is provided for convenience, see :ref:`this |
| 92 | section<automatic-compliance-checking>` for more details. |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 93 | |
Sandrine Bailleux | 75804e5 | 2020-08-12 11:29:46 +0200 | [diff] [blame] | 94 | - Where appropriate, please update the documentation. |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 95 | |
Sandrine Bailleux | 75804e5 | 2020-08-12 11:29:46 +0200 | [diff] [blame] | 96 | - Consider whether the :ref:`Porting Guide`, :ref:`Firmware Design` document |
| 97 | or other in-source documentation needs updating. |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 98 | |
Sandrine Bailleux | d314734 | 2020-05-12 10:36:05 +0200 | [diff] [blame] | 99 | - If you are submitting new files that you intend to be the code owner for |
| 100 | (for example, a new platform port), then also update the |
| 101 | :ref:`code owners` file. |
Sandrine Bailleux | 75804e5 | 2020-08-12 11:29:46 +0200 | [diff] [blame] | 102 | |
| 103 | - For topics with multiple commits, you should make all documentation changes |
| 104 | (and nothing else) in the last commit of the series. Otherwise, include |
| 105 | the documentation changes within the single commit. |
| 106 | |
Sandrine Bailleux | 398b188 | 2020-08-17 08:52:33 +0200 | [diff] [blame] | 107 | .. _copyright-license-guidance: |
| 108 | |
Sandrine Bailleux | 75804e5 | 2020-08-12 11:29:46 +0200 | [diff] [blame] | 109 | - Ensure that each changed file has the correct copyright and license |
| 110 | information. Files that entirely consist of contributions to this project |
| 111 | should have a copyright notice and BSD-3-Clause SPDX license identifier of |
| 112 | the form as shown in :ref:`license`. Files that contain changes to imported |
| 113 | Third Party IP files should retain their original copyright and license |
| 114 | notices. |
| 115 | |
| 116 | For significant contributions you may add your own copyright notice in the |
| 117 | following format: |
| 118 | |
| 119 | :: |
| 120 | |
| 121 | Portions copyright (c) [XXXX-]YYYY, <OWNER>. All rights reserved. |
| 122 | |
| 123 | where XXXX is the year of first contribution (if different to YYYY) and YYYY |
| 124 | is the year of most recent contribution. <OWNER> is your name or your company |
| 125 | name. |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 126 | |
Sandrine Bailleux | e0cb189 | 2020-08-14 15:58:50 +0200 | [diff] [blame] | 127 | - Ensure that each patch in the patch series compiles in all supported |
| 128 | configurations. Patches which do not compile will not be merged. |
| 129 | |
Louis Mayencourt | 72ef3d4 | 2019-03-22 11:47:22 +0000 | [diff] [blame] | 130 | - Please test your changes. As a minimum, ensure that Linux boots on the |
Paul Beesley | d2fcc4e | 2019-05-29 13:59:40 +0100 | [diff] [blame] | 131 | Foundation FVP. See :ref:`Arm Fixed Virtual Platforms (FVP)` for more |
| 132 | information. For more extensive testing, consider running the `TF-A Tests`_ |
| 133 | against your patches. |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 134 | |
Sandrine Bailleux | e0cb189 | 2020-08-14 15:58:50 +0200 | [diff] [blame] | 135 | - Ensure that all CI automated tests pass. Failures should be fixed. They might |
| 136 | block a patch, depending on how critical they are. |
| 137 | |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 138 | Submitting Changes |
| 139 | ------------------ |
| 140 | |
Sandrine Bailleux | e0cb189 | 2020-08-14 15:58:50 +0200 | [diff] [blame] | 141 | - Submit your changes for review at https://review.trustedfirmware.org |
| 142 | targeting the ``integration`` branch. |
Louis Mayencourt | 72ef3d4 | 2019-03-22 11:47:22 +0000 | [diff] [blame] | 143 | |
Sandrine Bailleux | e0cb189 | 2020-08-14 15:58:50 +0200 | [diff] [blame] | 144 | - Add reviewers for your patch: |
Louis Mayencourt | 72ef3d4 | 2019-03-22 11:47:22 +0000 | [diff] [blame] | 145 | |
Sandrine Bailleux | e0cb189 | 2020-08-14 15:58:50 +0200 | [diff] [blame] | 146 | - At least one code owner for each module modified by the patch. See the list |
| 147 | of modules and their :ref:`code owners`. |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 148 | |
Sandrine Bailleux | e0cb189 | 2020-08-14 15:58:50 +0200 | [diff] [blame] | 149 | - At least one maintainer. See the list of :ref:`maintainers`. |
Louis Mayencourt | 72ef3d4 | 2019-03-22 11:47:22 +0000 | [diff] [blame] | 150 | |
Sandrine Bailleux | e0cb189 | 2020-08-14 15:58:50 +0200 | [diff] [blame] | 151 | - If some module has no code owner, try to identify a suitable (non-code |
| 152 | owner) reviewer. Running ``git blame`` on the module's source code can |
| 153 | help, as it shows who has been working the most recently on this area of |
| 154 | the code. |
Louis Mayencourt | 72ef3d4 | 2019-03-22 11:47:22 +0000 | [diff] [blame] | 155 | |
Sandrine Bailleux | e0cb189 | 2020-08-14 15:58:50 +0200 | [diff] [blame] | 156 | Alternatively, if it is impractical to identify such a reviewer, you might |
| 157 | send an email to the `TF-A mailing list`_ to broadcast your review request |
| 158 | to the community. |
| 159 | |
| 160 | Note that self-reviewing a patch is prohibited, even if the patch author is |
| 161 | the only code owner of a module modified by the patch. Getting a second pair |
| 162 | of eyes on the code is essential to keep up with the quality standards the |
| 163 | project aspires to. |
| 164 | |
| 165 | - The changes will then undergo further review by the designated people. Any |
| 166 | review comments will be made directly on your patch. This may require you to |
| 167 | do some rework. For controversial changes, the discussion might be moved to |
| 168 | the `TF-A mailing list`_ to involve more of the community. |
Louis Mayencourt | 72ef3d4 | 2019-03-22 11:47:22 +0000 | [diff] [blame] | 169 | |
| 170 | Refer to the `Gerrit Uploading Changes documentation`_ for more details. |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 171 | |
Sandrine Bailleux | e0cb189 | 2020-08-14 15:58:50 +0200 | [diff] [blame] | 172 | - The patch submission rules are the following. For a patch to be approved |
| 173 | and merged in the tree, it must get: |
| 174 | |
| 175 | - One ``Code-Owner-Review+1`` for each of the modules modified by the patch. |
| 176 | - A ``Maintainer-Review+1``. |
| 177 | |
| 178 | In the case where a code owner could not be found for a given module, |
| 179 | ``Code-Owner-Review+1`` is substituted by ``Code-Review+1``. |
| 180 | |
| 181 | In addition to these various code review labels, the patch must also get a |
| 182 | ``Verified+1``. This is usually set by the Continuous Integration (CI) bot |
| 183 | when all automated tests passed on the patch. Sometimes, some of these |
| 184 | automated tests may fail for reasons unrelated to the patch. In this case, |
| 185 | the maintainers might (after analysis of the failures) override the CI bot |
| 186 | score to certify that the patch has been correctly tested. |
| 187 | |
| 188 | In the event where the CI system lacks proper tests for a patch, the patch |
| 189 | author or a reviewer might agree to perform additional manual tests |
| 190 | in their review and the reviewer incorporates the review of the additional |
| 191 | testing in the ``Code-Review+1`` or ``Code-Owner-Review+1`` as applicable to |
| 192 | attest that the patch works as expected. Where possible additional tests should |
| 193 | be added to the CI system as a follow up task. For example, for a |
| 194 | platform-dependent patch where the said platform is not available in the CI |
| 195 | system's board farm. |
| 196 | |
Paul Beesley | f864067 | 2019-04-12 14:19:42 +0100 | [diff] [blame] | 197 | - When the changes are accepted, the :ref:`maintainers` will integrate them. |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 198 | |
Paul Beesley | f864067 | 2019-04-12 14:19:42 +0100 | [diff] [blame] | 199 | - Typically, the :ref:`maintainers` will merge the changes into the |
Louis Mayencourt | 72ef3d4 | 2019-03-22 11:47:22 +0000 | [diff] [blame] | 200 | ``integration`` branch. |
Sandrine Bailleux | e0cb189 | 2020-08-14 15:58:50 +0200 | [diff] [blame] | 201 | |
Louis Mayencourt | 72ef3d4 | 2019-03-22 11:47:22 +0000 | [diff] [blame] | 202 | - If the changes are not based on a sufficiently-recent commit, or if they |
Paul Beesley | f864067 | 2019-04-12 14:19:42 +0100 | [diff] [blame] | 203 | cannot be automatically rebased, then the :ref:`maintainers` may rebase it |
Sandrine Bailleux | d314734 | 2020-05-12 10:36:05 +0200 | [diff] [blame] | 204 | on the ``integration`` branch or ask you to do so. |
Sandrine Bailleux | e0cb189 | 2020-08-14 15:58:50 +0200 | [diff] [blame] | 205 | |
Louis Mayencourt | 72ef3d4 | 2019-03-22 11:47:22 +0000 | [diff] [blame] | 206 | - After final integration testing, the changes will make their way into the |
Sandrine Bailleux | d314734 | 2020-05-12 10:36:05 +0200 | [diff] [blame] | 207 | ``master`` branch. If a problem is found during integration, the |
| 208 | :ref:`maintainers` will request your help to solve the issue. They may |
| 209 | revert your patches and ask you to resubmit a reworked version of them or |
| 210 | they may ask you to provide a fix-up patch. |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 211 | |
Julius Werner | cece91a | 2019-04-18 16:47:46 -0700 | [diff] [blame] | 212 | Binary Components |
| 213 | ----------------- |
| 214 | |
| 215 | - Platforms may depend on binary components submitted to the `Trusted Firmware |
| 216 | binary repository`_ if they require code that the contributor is unable or |
| 217 | unwilling to open-source. This should be used as a rare exception. |
| 218 | - All binary components must follow the contribution guidelines (in particular |
| 219 | licensing rules) outlined in the `readme.rst <tf-binaries-readme_>`_ file of |
| 220 | the binary repository. |
| 221 | - Binary components must be restricted to only the specific functionality that |
| 222 | cannot be open-sourced and must be linked into a larger open-source platform |
| 223 | port. The majority of the platform port must still be implemented in open |
| 224 | source. Platform ports that are merely a thin wrapper around a binary |
| 225 | component that contains all the actual code will not be accepted. |
| 226 | - Only platform port code (i.e. in the ``plat/<vendor>`` directory) may rely on |
| 227 | binary components. Generic code must always be fully open-source. |
| 228 | |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 229 | -------------- |
| 230 | |
Paul Beesley | 07f0a31 | 2019-05-16 13:33:18 +0100 | [diff] [blame] | 231 | *Copyright (c) 2013-2020, Arm Limited and Contributors. All rights reserved.* |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 232 | |
Chris Kay | 98b26e3 | 2020-12-09 16:52:03 +0000 | [diff] [blame] | 233 | .. _Conventional Commits: https://www.conventionalcommits.org/en/v1.0.0 |
Louis Mayencourt | 72ef3d4 | 2019-03-22 11:47:22 +0000 | [diff] [blame] | 234 | .. _developer.trustedfirmware.org: https://developer.trustedfirmware.org |
Sandrine Bailleux | d314734 | 2020-05-12 10:36:05 +0200 | [diff] [blame] | 235 | .. _review.trustedfirmware.org: https://review.trustedfirmware.org |
Louis Mayencourt | 72ef3d4 | 2019-03-22 11:47:22 +0000 | [diff] [blame] | 236 | .. _issue: https://developer.trustedfirmware.org/project/board/1/ |
John Tsichritzis | 2fd3d92 | 2019-05-28 13:13:39 +0100 | [diff] [blame] | 237 | .. _Trusted Firmware-A: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git |
Douglas Raillard | d7c21b7 | 2017-06-28 15:23:03 +0100 | [diff] [blame] | 238 | .. _Git guidelines: http://git-scm.com/book/ch5-2.html |
Louis Mayencourt | 72ef3d4 | 2019-03-22 11:47:22 +0000 | [diff] [blame] | 239 | .. _Gerrit Uploading Changes documentation: https://review.trustedfirmware.org/Documentation/user-upload.html |
| 240 | .. _Gerrit Signed-off-by Lines guidelines: https://review.trustedfirmware.org/Documentation/user-signedoffby.html |
| 241 | .. _Gerrit Change-Ids documentation: https://review.trustedfirmware.org/Documentation/user-changeid.html |
Sandrine Bailleux | d314734 | 2020-05-12 10:36:05 +0200 | [diff] [blame] | 242 | .. _TF-A Tests: https://trustedfirmware-a-tests.readthedocs.io |
Julius Werner | cece91a | 2019-04-18 16:47:46 -0700 | [diff] [blame] | 243 | .. _Trusted Firmware binary repository: https://review.trustedfirmware.org/admin/repos/tf-binaries |
| 244 | .. _tf-binaries-readme: https://git.trustedfirmware.org/tf-binaries.git/tree/readme.rst |
Sandrine Bailleux | d314734 | 2020-05-12 10:36:05 +0200 | [diff] [blame] | 245 | .. _TF-A mailing list: https://lists.trustedfirmware.org/mailman/listinfo/tf-a |