diff --git a/doc/source/contributor/gerrit.rst b/doc/source/contributor/gerrit.rst index 146940465a..d44c458fc6 100644 --- a/doc/source/contributor/gerrit.rst +++ b/doc/source/contributor/gerrit.rst @@ -1,3 +1,5 @@ +.. _code-reviews-with-gerrit: + Code Reviews with Gerrit ======================== diff --git a/doc/source/contributor/index.rst b/doc/source/contributor/index.rst index 0b6c73a7ec..0b06f6da70 100644 --- a/doc/source/contributor/index.rst +++ b/doc/source/contributor/index.rst @@ -58,6 +58,7 @@ Other Resources launchpad gerrit + manila-review-policy API Reference ------------- diff --git a/doc/source/contributor/manila-review-policy.rst b/doc/source/contributor/manila-review-policy.rst new file mode 100644 index 0000000000..5c681b8453 --- /dev/null +++ b/doc/source/contributor/manila-review-policy.rst @@ -0,0 +1,107 @@ +.. _manila-review-policy: + +Manila team code review policy +============================== + +Peer code review and the OpenStack Way +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Manila adheres to the `OpenStack code review policy and guidelines +`_. +Similar to other projects hosted on `git.openstack.org `_, all of manila's code is curated and maintained by a small +group of individuals called the "core team". The `primary core team +`_ +consists of members from diverse affiliations. There are special core teams +such as the `manila release core team `_ and the `manila stable maintenance core +team `_ that +have specific roles as the names suggest. + +To make a code change in openstack/manila or any of the associated code +repositories (openstack/manila-image-elements, openstack/manila-specs, +openstack/manila-tempest-plugin, openstack/manila-test-image, +openstack/manila-ui and openstack/python-manilaclient), contributors need to +follow the :ref:`Code Submission Process ` and +upload their code on the `OpenStack Gerrit `_ +website. They can then seek reviews by adding individual members of the +`manila core team `_ or alert the entire core team by inviting the Gerrit group +"manila-core" to the review. Anyone with a membership to the OpenStack +Gerrit system may review the code change. However, only the core team can +accept and merge the code change. Reviews from contributors outside the core +team are encouraged. Reviewing code meticulously and often is a +pre-requisite for contributors aspiring to join the core reviewer team. + +One or more core reviewers will take cognizance of the contribution and +provide feedback, or accept the code. For the submission to be accepted, it +will need a minimum of one Code-Review:+2 and one Workflow:+1 votes, along +with getting a Verified:+1 vote from the CI system. If no core reviewer pays +attention to a code submission, feel free to remind the team on the +#openstack-manila IRC channel on irc.freenode.com. [#]_ [#]_ + +Core code review guidelines +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +By convention rather than rule, we require that a minimum of two code +reviewers provide a Code-Review:+2 vote on each code submission before it is +given a Workflow:+1 vote. Having two core reviewers approve a change adds +diverse perspective, and is extremely valuable in case of: + +- Feature changes in the manila service stack +- Changes to configuration options +- Addition of new tests or significant test bug-fixes in manila-tempest-plugin +- New features to manila-ui, manila-test-image, manila-image-elements +- Bug fixes + +Trivial changes +--------------- +Trivial changes are: + +- Continuous Integration (CI) system break-fixes that are simple, + i.e.: + + - No job or test is being deleted + - Change does not break third-party CI + +- Documentation changes, especially typographical fixes and grammar + corrections. +- Automated changes generated by tooling - translations, lower-requirements + changes, etc. + +We do not need two core reviewers to approve trivial changes. + +Affiliation of core reviewers +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Previously, the manila core team informally enforced a code review +convention that each code change be reviewed and merged by +reviewers of different affiliations. This was followed because the +OpenStack Technical Committee used the diversity of +affiliation of the core reviewer team as a metric for maturity of the +project. However, since the Rocky release cycle, the TC has changed its view +on the subject [#]_ [#]_. We believe this is a step in the right +direction. + +While there is no strict requirement that two core reviewers accepting +a code change have different affiliations. Other things being equal, we will +continue to informally encourage organizational diversity by having core +reviewers from different organizations. Core reviewers have the professional +responsibility of avoiding conflicts of interest. + +Vendor code and review +~~~~~~~~~~~~~~~~~~~~~~ +All code in the manila repositories is open-source and anyone can submit +changes to these repositories as long as they seek to improve the code base. +Manila supports over 30 vendor storage systems, and many of these vendors +participate in the development and maintenance of their drivers. To the +extent possible, core reviewers will seek out driver maintainer feedback on +code changes pertaining to vendor integrations. + + +References +~~~~~~~~~~ + +.. [#] Getting started with IRC: https://docs.openstack.org/contributors/common/irc.html +.. [#] IRC guidelines: https://docs.openstack.org/infra/manual/irc.html +.. [#] TC Report 18-28: https://anticdent.org/tc-report-18-28.html +.. [#] TC vote to remove team diversity tags: https://review.openstack.org/#/c/579870/