summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorZuul <zuul@review.openstack.org>2019-01-02 14:52:36 +0000
committerGerrit Code Review <review@openstack.org>2019-01-02 14:52:36 +0000
commitd144bf7593241ada3982faa9fdb60279e5d7309c (patch)
treef358010ad7cdc553ef735a5a380de3c0927ed928
parent13d7fe9722138bce845e265d774c5b7c86d9d3c4 (diff)
parent39fe5dc0bcbd710fcfc935f4e2c5055c13c17819 (diff)
Merge "[DevRef] Add code review guideline"
-rw-r--r--doc/source/contributor/gerrit.rst2
-rw-r--r--doc/source/contributor/index.rst1
-rw-r--r--doc/source/contributor/manila-review-policy.rst107
3 files changed, 110 insertions, 0 deletions
diff --git a/doc/source/contributor/gerrit.rst b/doc/source/contributor/gerrit.rst
index 1469404..d44c458 100644
--- a/doc/source/contributor/gerrit.rst
+++ b/doc/source/contributor/gerrit.rst
@@ -1,3 +1,5 @@
1.. _code-reviews-with-gerrit:
2
1Code Reviews with Gerrit 3Code Reviews with Gerrit
2======================== 4========================
3 5
diff --git a/doc/source/contributor/index.rst b/doc/source/contributor/index.rst
index 0b6c73a..0b06f6d 100644
--- a/doc/source/contributor/index.rst
+++ b/doc/source/contributor/index.rst
@@ -58,6 +58,7 @@ Other Resources
58 58
59 launchpad 59 launchpad
60 gerrit 60 gerrit
61 manila-review-policy
61 62
62API Reference 63API Reference
63------------- 64-------------
diff --git a/doc/source/contributor/manila-review-policy.rst b/doc/source/contributor/manila-review-policy.rst
new file mode 100644
index 0000000..5c681b8
--- /dev/null
+++ b/doc/source/contributor/manila-review-policy.rst
@@ -0,0 +1,107 @@
1.. _manila-review-policy:
2
3Manila team code review policy
4==============================
5
6Peer code review and the OpenStack Way
7~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
8
9Manila adheres to the `OpenStack code review policy and guidelines
10<https://docs.openstack.org/infra/manual/developers.html#peer-review>`_.
11Similar to other projects hosted on `git.openstack.org <http://git.openstack
12.org/cgit>`_, all of manila's code is curated and maintained by a small
13group of individuals called the "core team". The `primary core team
14<https://review.openstack.org/#/admin/groups/213,members>`_
15consists of members from diverse affiliations. There are special core teams
16such as the `manila release core team <https://review.openstack
17.org/#/admin/groups/215,members>`_ and the `manila stable maintenance core
18team <https://review.openstack.org/#/admin/groups/1099,members>`_ that
19have specific roles as the names suggest.
20
21To make a code change in openstack/manila or any of the associated code
22repositories (openstack/manila-image-elements, openstack/manila-specs,
23openstack/manila-tempest-plugin, openstack/manila-test-image,
24openstack/manila-ui and openstack/python-manilaclient), contributors need to
25follow the :ref:`Code Submission Process <code-reviews-with-gerrit>` and
26upload their code on the `OpenStack Gerrit <https://review.openstack.org>`_
27website. They can then seek reviews by adding individual members of the
28`manila core team <https://review.openstack.org/#/admin/groups/213,
29members>`_ or alert the entire core team by inviting the Gerrit group
30"manila-core" to the review. Anyone with a membership to the OpenStack
31Gerrit system may review the code change. However, only the core team can
32accept and merge the code change. Reviews from contributors outside the core
33team are encouraged. Reviewing code meticulously and often is a
34pre-requisite for contributors aspiring to join the core reviewer team.
35
36One or more core reviewers will take cognizance of the contribution and
37provide feedback, or accept the code. For the submission to be accepted, it
38will need a minimum of one Code-Review:+2 and one Workflow:+1 votes, along
39with getting a Verified:+1 vote from the CI system. If no core reviewer pays
40attention to a code submission, feel free to remind the team on the
41#openstack-manila IRC channel on irc.freenode.com. [#]_ [#]_
42
43Core code review guidelines
44~~~~~~~~~~~~~~~~~~~~~~~~~~~
45
46By convention rather than rule, we require that a minimum of two code
47reviewers provide a Code-Review:+2 vote on each code submission before it is
48given a Workflow:+1 vote. Having two core reviewers approve a change adds
49diverse perspective, and is extremely valuable in case of:
50
51- Feature changes in the manila service stack
52- Changes to configuration options
53- Addition of new tests or significant test bug-fixes in manila-tempest-plugin
54- New features to manila-ui, manila-test-image, manila-image-elements
55- Bug fixes
56
57Trivial changes
58---------------
59Trivial changes are:
60
61- Continuous Integration (CI) system break-fixes that are simple,
62 i.e.:
63
64 - No job or test is being deleted
65 - Change does not break third-party CI
66
67- Documentation changes, especially typographical fixes and grammar
68 corrections.
69- Automated changes generated by tooling - translations, lower-requirements
70 changes, etc.
71
72We do not need two core reviewers to approve trivial changes.
73
74Affiliation of core reviewers
75~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
76Previously, the manila core team informally enforced a code review
77convention that each code change be reviewed and merged by
78reviewers of different affiliations. This was followed because the
79OpenStack Technical Committee used the diversity of
80affiliation of the core reviewer team as a metric for maturity of the
81project. However, since the Rocky release cycle, the TC has changed its view
82on the subject [#]_ [#]_. We believe this is a step in the right
83direction.
84
85While there is no strict requirement that two core reviewers accepting
86a code change have different affiliations. Other things being equal, we will
87continue to informally encourage organizational diversity by having core
88reviewers from different organizations. Core reviewers have the professional
89responsibility of avoiding conflicts of interest.
90
91Vendor code and review
92~~~~~~~~~~~~~~~~~~~~~~
93All code in the manila repositories is open-source and anyone can submit
94changes to these repositories as long as they seek to improve the code base.
95Manila supports over 30 vendor storage systems, and many of these vendors
96participate in the development and maintenance of their drivers. To the
97extent possible, core reviewers will seek out driver maintainer feedback on
98code changes pertaining to vendor integrations.
99
100
101References
102~~~~~~~~~~
103
104.. [#] Getting started with IRC: https://docs.openstack.org/contributors/common/irc.html
105.. [#] IRC guidelines: https://docs.openstack.org/infra/manual/irc.html
106.. [#] TC Report 18-28: https://anticdent.org/tc-report-18-28.html
107.. [#] TC vote to remove team diversity tags: https://review.openstack.org/#/c/579870/