docs: Add information on test removal/interop to REVIEWING
This patch set adds information on test removal, relocation,
renaming to REVIEWING because it is important that such
actions do not break interop. Interop is closely tied to
Tempest because it directly references Tempest tests that
are not only expected to exist, but to also work.
The same is true of breaking blacklist/whitelist
references to Tempest tests, this is also included
in the new documentation section.
It is important that there be REVIEWING guidelines
in place to assist reviewers understand this importance.
Also references are included for defcore/interop to help
users find more information on these topics.
Currently interop is only mentioned in 1 place in Tempest [0]
and yet there is little information about it. This patchset
aims to make it easier to find more information about it
for reviewers and users alike.
[0] http://git.openstack.org/cgit/openstack/tempest/tree/doc/source/test_removal.rst
Change-Id: Ifbde674b42355077fcd8daa07be8be1248b77b0f
diff --git a/REVIEWING.rst b/REVIEWING.rst
index a880181..8a1e152 100644
--- a/REVIEWING.rst
+++ b/REVIEWING.rst
@@ -99,6 +99,39 @@
scenario tests this is up to the reviewers discretion whether a docstring is
required or not.
+
+Test Removal and Refactoring
+----------------------------
+Make sure that any test that is renamed, relocated (e.g. moved to another
+class), or removed does not belong to the `interop`_ testing suite -- which
+includes a select suite of Tempest tests for the purposes of validating that
+OpenStack vendor clouds are interoperable -- or a project's `whitelist`_ or
+`blacklist`_ files.
+
+It is of critical importance that no interop, whitelist or blacklist test
+reference be broken by a patch set introduced to Tempest that renames,
+relocates or removes a referenced test.
+
+Please check the existence of code which references Tempest tests with:
+http://codesearch.openstack.org/
+
+Interop
+^^^^^^^
+Make sure that modifications to an `interop`_ test are backwards-compatible.
+This means that code modifications to tests should not undermine the quality of
+the validation currently performed by the test or significantly alter the
+behavior of the test.
+
+Removal
+^^^^^^^
+Reference the :ref:`test-removal` guidelines for understanding best practices
+associated with test removal.
+
+.. _interop: https://www.openstack.org/brand/interop
+.. _whitelist: https://docs.openstack.org/tempest/latest/run.html#test-selection
+.. _blacklist: https://docs.openstack.org/tempest/latest/run.html#test-selection
+
+
Release Notes
-------------
Release notes are how we indicate to users and other consumers of Tempest what
@@ -113,16 +146,18 @@
.. _reno: https://docs.openstack.org/reno/latest/
+
Deprecated Code
---------------
Sometimes we have some bugs in deprecated code. Basically, we leave it. Because
we don't need to maintain it. However, if the bug is critical, we might need to
fix it. When it will happen, we will deal with it on a case-by-case basis.
+
When to approve
---------------
* Every patch needs two +2s before being approved.
-* Its ok to hold off on an approval until a subject matter expert reviews it
+* It's ok to hold off on an approval until a subject matter expert reviews it
* If a patch has already been approved but requires a trivial rebase to merge,
you do not have to wait for a second +2, since the patch has already had
two +2s.