From ca4febac0cae2208bbdd730dc771b59bd292347b Mon Sep 17 00:00:00 2001 From: Sandrine Bailleux <sandrine.bailleux@arm.com> Date: Thu, 25 May 2023 15:46:01 +0200 Subject: [PATCH] docs: consolidate code review process documentation From the page listing the maintainers and code owners [1], add a link to the code review guidelines page [2], which in turn has a link to the tf.org code review process [3]. Before that patch, both pages [1] and [2] had a link to [3]. Hopefully, this change will guide the reader better so they don't miss out on any information. Additionally, move some of the information from the top of page [1] into page [2] and add extra details about the code review process used in TF-A and how that get translated in Gerrit. [1] https://trustedfirmware-a.readthedocs.io/en/latest/about/maintainers.html [2] https://trustedfirmware-a.readthedocs.io/en/latest/process/code-review-guidelines.html [3] https://developer.trustedfirmware.org/w/collaboration/project-maintenance-process/ Signed-off-by: Sandrine Bailleux <sandrine.bailleux@arm.com> Change-Id: I56562a72443f03fff16077dadc411ef4ee78666d --- docs/about/maintainers.rst | 11 +++----- docs/process/code-review-guidelines.rst | 37 +++++++++++++++++++------ 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/docs/about/maintainers.rst b/docs/about/maintainers.rst index 0287d6cee4..accb01a367 100644 --- a/docs/about/maintainers.rst +++ b/docs/about/maintainers.rst @@ -2,12 +2,11 @@ Project Maintenance =================== Trusted Firmware-A (TF-A) is an open governance community project. All -contributions are ultimately merged by the maintainers listed below. Technical -ownership of most parts of the codebase falls on the code owners listed -below. An acknowledgement from these code owners is required before the -maintainers merge a contribution. +contributions are reviewed and merged by the community members listed below. -More details may be found in the `Project Maintenance Process`_ document. +For more details on the roles of `maintainers`, `code owners` and general +information about code reviews in TF-A project, please refer to the :ref:`Code +Review Guidelines`. .. |M| replace:: **Mail** .. |G| replace:: **GitHub ID** @@ -980,5 +979,3 @@ Conventional Changelog Extensions .. _bytefire: https://github.com/bytefire .. _rupsin01: https://github.com/rupsin01 .. _jimmy-brisson: https://github.com/theotherjimmy - -.. _Project Maintenance Process: https://developer.trustedfirmware.org/w/collaboration/project-maintenance-process/ diff --git a/docs/process/code-review-guidelines.rst b/docs/process/code-review-guidelines.rst index 67a211f753..ccdd110e10 100644 --- a/docs/process/code-review-guidelines.rst +++ b/docs/process/code-review-guidelines.rst @@ -1,11 +1,6 @@ Code Review Guidelines ====================== -This document provides TF-A specific details about the project's code review -process. It should be read in conjunction with the `Project Maintenance -Process`_, which it supplements. - - Why do we do code reviews? -------------------------- @@ -23,8 +18,34 @@ Code reviews are meant to benefit everyone through team work. It is not about unfairly criticizing or belittling the work of any contributor. -Good practices --------------- +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: @@ -211,6 +232,6 @@ concerns, questions, or any other type of blocking comment, they should set -------------- -*Copyright (c) 2020, Arm Limited. All rights reserved.* +*Copyright (c) 2020-2023, Arm Limited. All rights reserved.* .. _Project Maintenance Process: https://developer.trustedfirmware.org/w/collaboration/project-maintenance-process/ -- GitLab