865aac4ace
OpenStack community is moving irc servers [1][2] by 31st May 2021. Since we refer to freenode by name in a couple of places, we can change those to point to OFTC, the new hub. [1] http://lists.openstack.org/pipermail/openstack-discuss/2021-May/022718.html [2] http://lists.openstack.org/pipermail/openstack-discuss/2021-May/022724.html Change-Id: Ic54ab0c40deb5ac0c519f9e4e54298eee6284c32 Signed-off-by: Goutham Pacha Ravi <gouthampravi@gmail.com>
108 lines
4.9 KiB
ReStructuredText
108 lines
4.9 KiB
ReStructuredText
.. _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
|
|
<https://docs.openstack.org/infra/manual/developers.html#peer-review>`_.
|
|
Similar to other projects hosted on `opendev.org <https://opendev.org>`_, all
|
|
of manila's code is curated and maintained by a small
|
|
group of individuals called the "core team". The `primary core team
|
|
<https://review.opendev.org/#/admin/groups/213,members>`_
|
|
consists of members from diverse affiliations. There are special core teams
|
|
such as the `manila release core team <https://review.opendev
|
|
.org/#/admin/groups/215,members>`_ and the `manila stable maintenance core
|
|
team <https://review.opendev.org/#/admin/groups/1099,members>`_ 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 <code-reviews-with-gerrit>` and
|
|
upload their code on the `OpenStack Gerrit <https://review.opendev.org>`_
|
|
website. They can then seek reviews by adding individual members of the
|
|
`manila core team <https://review.opendev.org/#/admin/groups/213,
|
|
members>`_ 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.oftc.net. [#]_ [#]_
|
|
|
|
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.opendev.org/#/c/579870/
|