openstack/neutron-lib
Code Issues Proposed changes

Update contributing doc

Browse Source 
Rehoming code into neutron-lib isn't always a straight
forward process. This patch updates our contributing
and index doc source to include a high-level workflow
for such situations.

Change-Id: I9c678b4e3223c5c44b840029102e73ab5111e268
This commit is contained in:
Boden R 2016-06-17 14:18:46 -06:00
parent efd7a3a5fa
commit 8b017d1e15
3 changed files with 172 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

140
doc/source/contributing.rst
View File

@ -2,3 +2,143 @@
Contributing
============
.. include:: ../../CONTRIBUTING.rst
As your code is subject to the `review guidelines <./review-guidelines.html>`_,
please take the time to familiarize yourself with those guidelines.
Rehoming Existing Code
----------------------
The checklist below aims to provide guidance for developers rehoming (moving) code into
neutron-lib. Rehoming approaches that fall outside the scope herein will need to be
considered on a case by case basis.
The rehoming workflow procedure has four main phases:
#. `Phase 1: Rehome`_ the code from neutron into neutron-lib.
#. `Phase 2: Enhance`_ the code in neutron-lib if necessary.
#. `Phase 3: Release`_ neutron-lib with the code so consumers can use it.
#. `Phase 4: Consume`_ by removing the rehomed code from its source and changing references
to use neutron-lib.
Phase 1: Rehome
~~~~~~~~~~~~~~~
#. Identify the chunk of code for rehoming. Applicable code includes common
classes/functions/modules/etc. that are consumed by networking project(s) outside of
neutron. Optimal consumption patterns of the code at hand must also be considered to
ensure the rehomed code addresses any technical debt. Finally, leave low-hanging
fruit for last and tackle the most commonly used code first. If you have any doubt
about the applicability of code for rehoming, reach out to one of the neutron core
developers before digging in.
#. Find and identify any unit tests for the code being rehomed. These unit tests
can often be moved into neutron-lib with minimal effort. After inspecting the
applicable unit tests, rewrite any that are non-optimal.
#. Search and understand the consumers of the code being rehomed. This must include other
networking projects in addition to neutron itself. At this point it may be determined
that the code should be refactored before it is consumed. There are a few common
strategies for refactoring, and the one chosen will depend on the nature of the code
at hand:
- Refactor/enhance the code as part of the initial neutron-lib patch. If this change
will be disruptive to consumers, clearly communicate the change via email list or
`meeting topic <https://wiki.openstack.org/wiki/Network/Meetings#Neutron-lib_and_planned_neutron_refactoring>`_.
- Leave the refactoring to the next (Enhance) phase. In this rehome phase, copy the code
as-is into a private module according to our `conventions <./conventions.html>`_. This
approach is slower, but may be necessary in some cases.
#. Understand existing work underway which may impact the rehomed code, for example,
in-flight patch sets that update the code being rehomed. In some cases it may make
sense to let the in-flight patch merge and solidify a bit before rehoming.
#. Prepare the code for neutron-lib. This may require replacing existing imports
with those provided by neutron-lib and/or rewriting/rearchitecting non-optimal
code (see above). The interfaces in the rehomed code are subject to our
`conventions <./conventions.html>`_.
#. Prepare the unit test code for neutron-lib. As indicated in the `review guidelines
<./review-guidelines.html>`_ we are looking for a high code coverage by tests. This may
require adding additional tests if neutron was lacking in coverage.
#. Submit and shepherd your patch through its neutron-lib review. Include a
`release note <http://docs.openstack.org/developer/reno/>`_ that describes the code's
old neutron location and new neutron-lib location. Also note that in some cases it makes
sense to prototype a change in a consumer project to better understand the impacts of
the change, which can be done using the ``Depends-On:`` approach described in the
`review guidelines <./review-guidelines.html>`_
Examples:
- `319769 <https://review.openstack.org/319769/>`_ brought over a number of common
utility functions as-is from neutron into a new package structure within neutron-lib.
- `253661 <https://review.openstack.org/253661/>`_ rehomed neutron callbacks into a
private package that's enhanced via `346554 <https://review.openstack.org/346554/>`_.
- `319386 <https://review.openstack.org/319386/>`_ rehomes a validator from neutron
into neutron-lib.
Phase 2: Enhance
~~~~~~~~~~~~~~~~
If the rehomed code is not applicable for enhancements and wasn't made private in Phase 1,
you can skip this step.
Develop and shepherd the enhancements to the private rehomed code applicable at this time.
Private APIs made public as part of this phase will also need
`release notes <http://docs.openstack.org/developer/reno/>`_ indicating the new public
functionality.
Examples:
- `346554 <https://review.openstack.org/346554/>`_ enhances the rehomed private callback
API in neutron-lib.
Phase 3: Release
~~~~~~~~~~~~~~~~
A new neutron-lib release can be cut at any time. You can also request a release by following
the README instructions in the `openstack/releases <https://github.com/openstack/releases>`_
project.
Once a release is cut, an openstack infra proposal bot will submit patches to the master branch
of all projects that consume neutron-lib to set the new release as the minimum requirement.
Someone from the neutron release team can bump `global requirements` (g-r); for example
`review 393600 <https://review.openstack.org/393600/>`_.
When the bot-proposed requirement patches have merged, your rehomed code can be consumed.
Phase 4: Consume
~~~~~~~~~~~~~~~~
It's critical that before you submit your patch to remove the rehomed code from its source that
you perform a diff between it and the rehomed version in neutron-lib to ensure nothing has
changed in the source. If something has changed in the source, you need to push and shepherd a
patch to neutron-lib with the difference(s) before proceeding with this consumption phase.
The following guidelines are intended to provide a smooth transition to the rehomed code
in neutron-lib and minimize impacts to subprojects consuming the rehomed code from its
source.
- If the change to consume the code from neutron-lib is widespread and/or "important",
introduce your intentions for the change via the Neutron team
`meeting slot <https://wiki.openstack.org/wiki/Network/Meetings#Neutron-lib_and_planned_neutron_refactoring>`_
for neutron-lib. Subsequently follow-up with an email to openstack-dev list using a
subject with ``[neutron] neutron-lib impact`` providing additional details as necessary.
Ideally we can identify the main impacted subprojects by
`grepping the OpenStack code <http://codesearch.openstack.org/>`_.
- Prepare a neutron core patch to remove and update the rehomed code from its source.
This can be done without a `debtcollector <http://docs.openstack.org/developer/debtcollector>`_
notice by following the steps here. In the patch's commit message include the ``NeutronLibImpact``
so that we can easily `query <https://review.openstack.org/#/q/status:open+message:%22NeutronLibImpact%22>`_
for such changes. Mark the patch as a work in progress with a -1 workflow vote.
- If the change is significant enough, it may warrant a "reference implementation" in an
impacted subproject to ensure the impacts are fully understood. Testing this
change can be done using the ``Depends-On:`` approach described in the
`review guidelines <./review-guidelines.html>`_.
Examples:
- `348472 <https://review.openstack.org/348472/>`_ removes a validator in neutron that
was rehomed to neutron-lib.

8
doc/source/index.rst
View File

@ -15,7 +15,13 @@ Welcome to Neutron Lib developer documentation!
===============================================
Neutron-lib is an OpenStack library project used by Neutron, Advanced Services,
and third-party projects to provide common functionality and remove duplication.
and third-party projects that aims to provide common functionality across all
such consumers. The library is developed with the following goals in mind:
- Decouple sub-projects from Neutron (i.e. no direct neutron imports in
sub-projects).
- Pay down Neutron technical debt via refactoring/re-architecting of
sub-optimal patterns in their respective neutron-lib implementation.
This document describes the library for contributors of the project, and assumes
that you are already familiar with Neutron from an `end-user perspective`_.

35
doc/source/review-guidelines.rst
View File

@ -8,7 +8,9 @@ When reviewing neutron-lib changes, please be aware:
criteria:
- Is all of the code shared? Don't move neutron-only code.
- Is the interface good, or does it need to be refactored?
- Is the interface good, or does it need to be refactored? If refactoring
is required it must be done before the public interface is released to
PyPI as once released it must follow our `conventions <./conventions.html>`_.
- Does it need new tests, specifically around the interface? We want
a global unit coverage greater than 90%, and a per-module coverage
greater than 80%. If neutron does not yet have a test, it needs to
@ -16,18 +18,31 @@ When reviewing neutron-lib changes, please be aware:
but any code or interface should have a unit test, if you cannot
tell for sure that it is not going to be traversed in some alternative
way (e.g. tempest/functional coverage).
- Is there a corresponding Depends-On review in neutron removing
this code, and adding backwards compatibility shims for Mitaka?
- Do the public APIs have their parameters and return values documented
using reStructuredText docstring format (see below)?
- In certain cases, it may be beneficial to determine how the neutron-lib
code changes impact neutron `master`. This can be done as follows:
- Publish a 'Do Not Merge' dummy patch to neutron that uses the code
changes proposed (or already in) neutron-lib. Make sure to mark this
neutron change as a 'DNM' (or 'WIP') and use -1 for workflow to indicate.
- Publish a change to neutron-lib that uses `Depends-On:` for the
dummy change in neutron; this pulls the neutron dummy change into the
neutron-lib gate job. For example
`386846 <https://review.openstack.org/386846/>`_ uses a dummy
neutron-lib patch to test code that already exists in neutron-lib
`master` whereas `346554 <https://review.openstack.org/346554/13>`_
tests the neutron-lib patch's code itself.
- View neutron-lib gate job results and repeat as necessary.
* Public APIs should be documented using `reST style docstrings <https://www.python.org/dev/peps/pep-0287/>`_
that include an overview as well as parameter and return documentation.
The format of docstrings can be found in the `OpenStack developer hacking docs <http://docs.openstack.org/developer/hacking/#docstrings>`_.
Note that public API documentation is a bonus, not a requirement.
* Public classes and methods must not be destructively changed without
following the full OpenStack deprecation path.
* Once public classes and methods are pushed to PyPI as part of a neutron-lib
release, they must not be destructively changed without following the full
OpenStack deprecation path.
For example, do not:
@ -40,14 +55,14 @@ When reviewing neutron-lib changes, please be aware:
- Add a second method with the new signature
- Add keyword arguments
* Removing the code from neutron should include a shim in neutron
for the sake of subprojects. Refer to neutron/common/exceptions.py
for an example. Please Use oslo's debtcollector library,
example: http://docs.openstack.org/developer/debtcollector/
The above implies that if you add something, we are stuck with that interface
for a long time, so be careful.
* Removing the code from neutron can be done without a temporary `debtcollector
<http://docs.openstack.org/developer/debtcollector>`_ notice by following
the steps described in the 'Consume' phase of the
`contributing doc <./contributing.html>`_.
* Any code that imports/uses the following python modules should not be
moved into neutron-lib: