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 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 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 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.oftc.net. [1] [2]
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 [3] [4]. 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.
