Adding upstream version 2.10.0+dfsg.upstream/2.10.0+dfsgupstream

Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>

1 files changed, 245 insertions, 0 deletions

diff --git a/docs/process/code-review-guidelines.rst b/docs/process/code-review-guidelines.rst
new file mode 100644
index 0000000..bd42811
--- /dev/null
+++ b/docs/process/code-review-guidelines.rst
@@ -0,0 +1,245 @@
+Code Review Guidelines
+======================
+
+Why do we do code reviews?
+--------------------------
+
+The main goal of code reviews is to improve the code quality. By reviewing each
+other's code, we can help catch issues that were missed by the author
+before they are integrated in the source tree. Different people bring different
+perspectives, depending on their past work, experiences and their current use
+cases of TF-A in their products.
+
+Code reviews also play a key role in sharing knowledge within the
+community. People with more expertise in one area of the code base can
+help those that are less familiar with it.
+
+Code reviews are meant to benefit everyone through team work. It is not about
+unfairly criticizing or belittling the work of any contributor.
+
+
+Overview of the code review process
+-----------------------------------
+
+All contributions to Trusted Firmware-A project are reviewed by the community to
+ensure they meet the project's expectations before they get merged, according to
+the `Project Maintenance Process`_ defined for all `Trusted Firmware` projects.
+
+Technical ownership of most parts of the codebase falls on the :ref:`code
+owners`. All patches are ultimately merged by the :ref:`maintainers`.
+
+Approval of a patch is tracked using Gerrit `labels`. For a patch to be merged,
+it must get all of the following votes:
+
+- At least one ``Code-Owner-Review+1`` up-vote, and no ``Code-Owner-Review-1``
+  down-vote.
+
+- At least one ``Maintainer-Review+1`` up-vote, and no ``Maintainer-Review-1``
+  down-vote.
+
+- ``Verified+1`` vote applied by the automated Continuous Integration (CI)
+  system.
+
+Note that, in some instances, the maintainers might give a waiver for some of
+the CI failures and manually override the ``Verified+1`` score.
+
+
+Good practices for all reviewers
+--------------------------------
+
+To ensure the code review gives the greatest possible benefit, participants in
+the project should:
+
+-  Be considerate of other people and their needs. Participants may be working
+   to different timescales, and have different priorities. Keep this in
+   mind - be gracious while waiting for action from others, and timely in your
+   actions when others are waiting for you.
+
+-  Review other people's patches where possible. The more active reviewers there
+   are, the more quickly new patches can be reviewed and merged. Contributing to
+   code review helps everyone in the long run, as it creates a culture of
+   participation which serves everyone's interests.
+
+
+Guidelines for patch contributors
+---------------------------------
+
+In addition to the rules outlined in the :ref:`Contributor's Guide`, as a patch
+contributor you are expected to:
+
+-  Answer all comments from people who took the time to review your
+   patches.
+
+-  Be patient and resilient. It is quite common for patches to go through
+   several rounds of reviews and rework before they get approved, especially
+   for larger features.
+
+   In the event that a code review takes longer than you would hope for, you
+   may try the following actions to speed it up:
+
+  -  Ping the reviewers on Gerrit or on the mailing list. If it is urgent,
+     explain why. Please remain courteous and do not abuse this.
+
+  -  If one code owner has become unresponsive, ask the other code owners for
+     help progressing the patch.
+
+  -  If there is only one code owner and they have become unresponsive, ask one
+     of the project maintainers for help.
+
+-  Do the right thing for the project, not the fastest thing to get code merged.
+
+   For example, if some existing piece of code - say a driver - does not quite
+   meet your exact needs, go the extra mile and extend the code with the missing
+   functionality you require - as opposed to copying the code into some other
+   directory to have the freedom to change it in any way. This way, your changes
+   benefit everyone and will be maintained over time.
+
+-  It is the patch-author's responsibility to respond to review comments within
+   21 days. In the event that the patch-author does not respond within this
+   timeframe, the maintainer is entitled to abandon the patch(es).
+   Patch author(s) may be busy with other priorities, causing a delay in
+   responding to active review comments after posting patch(es). In such a
+   situation, if the author's patch(es) is/are abandoned, they can restore
+   their work for review by resolving comments, merge-conflicts, and revising
+   their original submissions.
+
+Guidelines for all reviewers
+----------------------------
+
+There are no good or bad review comments. If you have any doubt about a patch or
+need some clarifications, it's better to ask rather than letting a potential
+issue slip. Examples of review comments could be:
+
+- Questions ("Why do you need to do this?", "What if X happens?")
+- Bugs ("I think you need a logical \|\| rather than a bitwise \|.")
+- Design issues ("This won't scale well when we introduce feature X.")
+- Improvements ("Would it be better if we did Y instead?")
+
+
+Guidelines for code owners
+--------------------------
+
+Code owners are listed on the :ref:`Project Maintenance<code owners>` page,
+along with the module(s) they look after.
+
+When reviewing a patch, code owners are expected to check the following:
+
+-  The patch looks good from a technical point of view. For example:
+
+  -  The structure of the code is clear.
+
+  -  It complies with the relevant standards or technical documentation (where
+     applicable).
+
+  -  It leverages existing interfaces rather than introducing new ones
+     unnecessarily.
+
+  -  It fits well in the design of the module.
+
+  -  It adheres to the security model of the project. In particular, it does not
+     increase the attack surface (e.g. new SMCs) without justification.
+
+-  The patch adheres to the TF-A :ref:`Coding Style`. The CI system should help
+   catch coding style violations.
+
+-  (Only applicable to generic code) The code is MISRA-compliant (see
+   :ref:`misra-compliance`). The CI system should help catch violations.
+
+-  Documentation is provided/updated (where applicable).
+
+-  The patch has had an appropriate level of testing. Testing details are
+   expected to be provided by the patch author. If they are not, do not hesitate
+   to request this information.
+
+-  All CI automated tests pass.
+
+If a code owner is happy with a patch, they should give their approval
+through the ``Code-Owner-Review+1`` label in Gerrit. If instead, they have
+concerns, questions, or any other type of blocking comment, they should set
+``Code-Owner-Review-1``.
+
+Code owners are expected to behave professionally and responsibly. Here are some
+guidelines for them:
+
+-  Once you are engaged in a review, make sure you stay involved until the patch
+   is merged. Rejecting a patch and going away is not very helpful. You are
+   expected to monitor the patch author's answers to your review comments,
+   answer back if needed and review new revisions of their patch.
+
+-  Provide constructive feedback. Just saying, "This is wrong, you should do X
+   instead." is usually not very helpful. The patch author is unlikely to
+   understand why you are requesting this change and might feel personally
+   attacked.
+
+-  Be mindful when reviewing a patch. As a code owner, you are viewed as
+   the expert for the relevant module. By approving a patch, you are partially
+   responsible for its quality and the effects it has for all TF-A users. Make
+   sure you fully understand what the implications of a patch might be.
+
+
+Guidelines for maintainers
+--------------------------
+
+Maintainers are listed on the :ref:`Project Maintenance<maintainers>` page.
+
+When reviewing a patch, maintainers are expected to check the following:
+
+-  The general structure of the patch looks good. This covers things like:
+
+   -  Code organization.
+
+   -  Files and directories, names and locations.
+
+      For example, platform code should be added under the ``plat/`` directory.
+
+   -  Naming conventions.
+
+      For example, platform identifiers should be properly namespaced to avoid
+      name clashes with generic code.
+
+   -  API design.
+
+-  Interaction of the patch with other modules in the code base.
+
+-  The patch aims at complying with any standard or technical documentation
+   that applies.
+
+-  New files must have the correct license and copyright headers. See :ref:`this
+   paragraph<copyright-license-guidance>` for more information. The CI system
+   should help catch files with incorrect or no copyright/license headers.
+
+-  There is no third party code or binary blobs with potential IP concerns.
+   Maintainers should look for copyright or license notices in code, and use
+   their best judgement. If they are unsure about a patch, they should ask
+   other maintainers for help.
+
+-  Generally speaking, new driver code should be placed in the generic
+   layer. There are cases where a driver has to stay into the platform layer but
+   this should be the exception, rather than the rule.
+
+-  Existing common drivers (in particular for Arm IPs like the GIC driver) should
+   not be copied into the platform layer to cater for platform quirks. This
+   type of code duplication hurts the maintainability of the project. The
+   duplicate driver is less likely to benefit from bug fixes and future
+   enhancements. In most cases, it is possible to rework a generic driver to
+   make it more flexible and fit slightly different use cases. That way, these
+   enhancements benefit everyone.
+
+-  When a platform specific driver really is required, the burden lies with the
+   patch author to prove the need for it. A detailed justification should be
+   posted via the commit message or on the mailing list.
+
+-  Before merging a patch, verify that all review comments have been addressed.
+   If this is not the case, encourage the patch author and the relevant
+   reviewers to resolve these together.
+
+If a maintainer is happy with a patch, they should give their approval
+through the ``Maintainer-Review+1`` label in Gerrit. If instead, they have
+concerns, questions, or any other type of blocking comment, they should set
+``Maintainer-Review-1``.
+
+--------------
+
+*Copyright (c) 2020-2023, Arm Limited. All rights reserved.*
+
+.. _Project Maintenance Process: https://developer.trustedfirmware.org/w/collaboration/project-maintenance-process/