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