openstack/openstack-manuals
Code Issues Proposed changes

[contributor] reorganize reviewing documentation chapter

Browse Source 
Also, migrated the review guidelines from
https://wiki.openstack.org/wiki/Documentation/ReviewGuidelines

Change-Id: I1d6d97b0826869834e4252cb2458f5bd03233c73
Implements: blueprint contributor-guide-reorg
This commit is contained in:
KATO Tomoyuki 2016-07-07 10:27:01 +09:00 committed by Lana Brindley
parent e850abef9d
commit 8c7fdb588e
2 changed files with 338 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

209
doc/contributor-guide/source/docs-review-guidelines.rst Normal file
View File

@ -0,0 +1,209 @@
=================
Review guidelines
=================
This section provides guidelines to improve the quality and speed of
the documentation review process.
Critique categories
~~~~~~~~~~~~~~~~~~~
Below is a list of the categories you will critique as the reviewer.
Objective
---------
* `Commit message <https://wiki.openstack.org/wiki/GitCommitMessages>`_
* Content
* Conventions
* Bug number or blueprint
* Patch
* Content
* Grammar
* :doc:`Style, phrasing, wording, and capitalization <writing-style>`
* Spelling
* :doc:`RST formatting <rst-conv>`
* Accuracy of the selected location
Subjective
----------
* Commit message
* Grammar
* Spelling
* Style, phrasing, and wording
* Patch
* Grammar
* Style, phrasing, and wording
* Technical accuracy
* Other suggestions
Scope
~~~~~
Try to keep reviews limited to the contents of the bug, contents of
the commit message, and changes made by the patch.
In other words, if the patch solves the stated problem, but there are
other improvements that could result from the patch, approve the patch
and file a subsequent bug, rather than -1'ing the patch with a comment
about improvements. This is why it is a good idea to write commit
messages that clearly define the scope of the patch.
Additional comments within the objective and subjective aims of a patch,
that would result in a -1, are appropriate.
Remember to consider if the change is related to the scope.
Consistency
~~~~~~~~~~~
* Mark all instances of an issue if one appears in a patch.
* If the author uploads a patch correcting your objective issue and
you find another instance that you didn't mark, comment on it and
score with a -1. Preferably, upload a patch to fix it.
* If the author uploads a patch correcting your subjective issue and
you find another instance that you didn't mark, comment on it and
score with a 0.
* If the author uploads a patch correcting your objective and/or
subjective issue and you find another objective issue, comment on
it and score with a -1. Preferably, upload a patch to fix it.
* If the author uploads a patch correcting your objective and/or
subjective issue and you find another subjective issue, comment on
it and score with a 0.
* If an issue appears that could affect other portions of a book,
provide appropriate comments, score the patch with a -1, and consider
mentioning your issue on the mailing list or in a meeting.
Example: A new service uses "key = value" in the configuration file
and all other services use "key=value" in their configuration files.
Both methods work, but the book should maintain consistency.
* If a patch has already received a -1, do not be discouraged from
checking on it, and add additional comments. This way, there will
be fewer patch sets and comments building up under one review.
Tagging additional reviewers
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In some cases, you should tag one or more people with interest in or
experience with the content of your patch to review it.
Q: How long should an author wait for reviews by these people?
A: For extremely busy projects with backlogs of over 400 patches,
wait two weeks at least. If you have difficulty getting a reviewer
for a particular project, use the `Cross Project Documentation Liaisons
<https://wiki.openstack.org/wiki/CrossProjectLiaisons#Documentation>`_
page to contact the documentation liaison for the project to get a reviewer.
The Documentation PTL can also assist in getting reviewers attention.
The waiting game
~~~~~~~~~~~~~~~~
After the first review with a -1 or 0 score, the author should update
the patch. Authors do not need to wait for a lengthy period of time.
Expect to leave some time for reviewers to check on a patch, however.
Consider that some reviewers are located in different timezones.
Core reviewers will in general review a patch within a few days.
If no review happens, feel free to ask on the #openstack-doc IRC channel.
Review scoring and approvals
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
* Scores available to contributors: -1, 0, +1
* Scores available to core reviewers: -2, -1, 0, +1, +2
* Approvals
A core reviewer can +2 score a patch with a +2 score from another core
reviewer and approve it.
If the change contains important or critical information,
the core reviewer should not approve it immediately, but provide a few days
for wider community audience to express their opinion, and suggest posting
to the documentation mailing list.
.. note::
If you find an issue with a patch that already has a +2 score from
another core reviewer, consider commenting on the issue and scoring the
patch with a 0 rather than scoring it with a -1.
Setting WorkInProgress (WIP) during review
------------------------------------------
The WIP tag tells potential reviewers to expect additional updates to
a patch before reviewing. Both the change's owner and any core reviewer
can set the WIP status:
* A change owner and core reviewers can set this tag on their own review
to mark that additional changes are still being made, and to avoid
unnecessary reviews while that happens.
This can be a great convenience to fellow reviewers. It allows the core
reviewer to politely send the message that the change needs additional
work while simultaneously removing it from the list of ready changes
until that happens.
To add the WIP tag:
#. Select a patch set.
#. Click the :guilabel:`Reply...` button.
#. Choose :guilabel:`-1 (Work In Progress)` from the :guilabel:`Workflow`
options.
#. Optional: enter comments.
#. Click :guilabel:`Post`.
This sets a ``-1`` and informs everyone that the patch is Work In Progress.
Abandoning patches
------------------
Core reviewers may abandon patches that receive a -2 review or
lack activity for at least four weeks to freshen the patch review queue.
The owner of a patch can also abandon it.
The owner and core reviewers can restore it again.
Patches by OpenStack Proposal Bot
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
There are a few proposal jobs set up that are run regularly:
* ``Imported Translations from Zanata``: Import of translated files from
the translation infrastructure.
This is run once a day (06:00 UTC) for each repository.
* ``Updated from openstack-manuals``: Import of glossary files from
openstack-manuals. This job is triggered whenever openstack-manuals has
merged and will propose a change if something has changed.
* ``Updated from global requirements``: Import of requirements.txt and
test-requirements.txt from the global requirements repository.
For all types of patches, any core can approve a patch if all the tests pass.
If the tests do not pass, vote ``-1`` on the patch, fix the problem and
wait for the next proposal run.
The proposal job will update the patch with the next run.
If you cannot fix the problem, ask for help on the mailing lists:
* openstack-i18n: translation failures
* openstack-doc: requirements failures, glossary sync failures,
and common content sync failures.
Considerations for documentation aligned with release cycles
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Beginning with milestone releases, shift focus to objective issues,
especially with new services and existing services with significant changes.
Only patches with significant subjective issues should receive a -1 score.
Otherwise, comment on subjective issues and score with a 0.
Beginning with release candidates, focus almost entirely on content issues.
Only comment on subjective issues if the patch should receive a -1 score
for objective issues.

240
doc/contributor-guide/source/docs-review.rst
View File

@ -4,16 +4,25 @@
Reviewing documentation Reviewing documentation
======================= =======================
.. toctree::
:maxdepth: 1
docs-review-guidelines.rst
OpenStack documentation is treated like code.
We follow the standard code review process.
To see what documentation changes are ready for review, use the To see what documentation changes are ready for review, use the
`Documentation Program Dashboard`_. It is organized in groupings based on `Documentation Program Dashboard <http://is.gd/openstackdocsreview>`_.
the audience for the documentation. To see current proposed changes, make It is organized in groupings based on the audience for the documentation.
sure you register and log into https://review.openstack.org. For more To see current proposed changes, make sure you register and
details on the review process, see `Code Review`_. log into https://review.openstack.org.
For more details on the review process, see `Code Review
<http://docs.openstack.org/infra/manual/developers.html#code-review>`_.
Repositories and core team Repositories and core team
~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~
The OpenStack Docs team is core for api-site, openstack-manuals, The OpenStack Documentation team is core for api-site, openstack-manuals,
openstackdocstheme, and openstack-doc-tools projects. openstackdocstheme, and openstack-doc-tools projects.
For the following repositories that are part of the Documentation program, For the following repositories that are part of the Documentation program,
@ -30,17 +39,115 @@ special rules apply:
The current list of docs cores for openstack-manuals can be found at The current list of docs cores for openstack-manuals can be found at
https://review.openstack.org/#/admin/groups/30,members. https://review.openstack.org/#/admin/groups/30,members.
Reviewing a documentation patch
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Before you proceed with reviewing patches, make sure to read carefully
the :doc:`Review Guidelines <docs-review-guidelines>` for documentation
and `Code Review Guidelines
<http://docs.openstack.org/infra/manual/developers.html#code-review>`_.
Once done, follow the steps below to submit a patch review.
#. Go to the `Documentation Program Dashboard
<http://is.gd/openstackdocsreview>`_.
#. Select a patch set.
#. Click a file that was uploaded to view the changes side by side.
#. If you see some inconsistencies or have questions to the patch owner,
you can also highlight the line or word in question, and press 'c'
on your keyboard, which enables commenting directly on that line or word.
Click :guilabel:`Save` button once you write a draft of your comment.
#. In the :guilabel:`Jenkins check` section, click the Jenkins ``checkbuild``
gate link (for the openstack-manuals, it is called
``gate-openstack-manuals-tox-doc-publish-checkbuild``) and review the
built manuals to see how the change will look on the web page. For a new
patch, it takes some time before Jenkins checks appear on the Gerrit
page. You can also `build the patch locally`_ if necessary.
#. Click :guilabel:`Reply` to vote and enter any comments about your review,
then click :guilabel:`Post`.
.. note::
A patch with WorkInProgress (WIP) status needs additional work
before review and possible approval. Therefore, you may skip such a patch
and review once it is ready. For more information, see `Work In Progress
<http://docs.openstack.org/infra/manual/core.html#work-in-progress>`_.
.. seealso::
`Peer Review
<http://docs.openstack.org/infra/manual/developers.html#peer-review>`_
.. _`build the patch locally`:
Building an existing patch locally
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Before proceeding, make sure you have all the necessary
:ref:`tools <setting_up_for_contribution>` installed and
set up for contribution.
To build a patch locally:
#. Change to the directory containing the appropriate repository.
.. code-block:: console
$ cd openstack-manuals
#. Create a local branch that contains the particular patch.
.. code-block:: console
$ git review -d PATCH_ID
Where the value of PATCH_ID is a Gerrit commit number.
You can find this number on the patch link,
``https://review.openstack.org/#/c/PATCH_ID``.
#. Build all the books that are affected by changes in the patch set:
.. code-block:: console
$ tox -e checkbuild
#. Find the build result in ``publish-docs/index.html``.
#. Review the source and the output. You can edit and update the patch:
#. Ensure that your edits adhere to the
:ref:`Writing Style <stg_writing_style>` for OpenStack documentation
and use standard U.S. English.
#. Once the build and new output are good to commit, run:
.. code-block:: console
$ git commit -a --amend
#. When the editor opens, update the commit message if necessary. But do
not add information on what your specific patch set changes. A reviewer
can use the Gerrit interface to see the difference between patches.
#. Save the changes if any, and exit the editor.
#. Send your patch to the existing review:
.. code-block:: console
$ git review
#. Leave a comment in Gerrit explaining the reason for the additional
changes.
Achieving a core reviewer status Achieving a core reviewer status
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Core reviewers are able to +2 and merge content into the projects they have Core reviewers are able to +2 and merge content into the projects they
the core status in. Core status is granted to those who have not only done a have the core status in. Core status is granted to **those who have not
lot of reviews, but who also have shown care and wisdom in those reviews. only done a sufficient quantity of reviews, but who also have shown care
Becoming a core reviewer also carries with it a responsibility: you are now and wisdom in those reviews**. Becoming a core reviewer also carries with
the *guardian of the gate*, and it is up to the core team to ensure that it a responsibility: you are now the **guardian of the gate**, and
nothing untoward gets through, without discouraging contributions. The core it is up to the core team to ensure that nothing unfavorable gets through,
reviewer's role is complex, and having a great core team is crucial to the without discouraging contributions.
success of any OpenStack project. The core reviewer's role is complex, and having a great core team is
crucial to the success of any OpenStack project.
With great power comes great responsibility. With great power comes great responsibility.
@ -58,108 +165,19 @@ nomination-based approach. This means a couple of things:
The process is: The process is:
- Every month (usually on the 1st), the documentation PTL draws the top 12 * Every month (usually on the 1st), the documentation PTL draws the top 12
names using these reports: names using these reports:
- http://russellbryant.net/openstack-stats/docs-reviewers-30.txt * http://russellbryant.net/openstack-stats/docs-reviewers-30.txt
- http://russellbryant.net/openstack-stats/docs-reviewers-90.txt * http://russellbryant.net/openstack-stats/docs-reviewers-90.txt
- http://stackalytics.com/?module=openstack-manuals&metric=commits * http://stackalytics.com/?module=openstack-manuals&metric=commits
- The PTL then consults the existing core team with a list of names to be * The PTL then consults the existing core team with a list of names to be
removed and added from/to the core list. Once an agreement is reached, the removed from and added to the core list. Once an agreement is reached,
changes are made and advertised to the main documentation mailing list. the changes are made and advertised to the main documentation mailing list.
Cores who are being removed will be contacted personally before removal. Cores who are being removed will be contacted personally before removal.
- Existing core team members can nominate a new core member at any time,
* Existing core team members can nominate a new core member at any time,
with a justification sent to the existing core team: with a justification sent to the existing core team:
openstack-doc-core@lists.launchpad.net. Three +1 votes from other existing openstack-doc-core@lists.launchpad.net. Three +1 votes from other existing
core team members must be achieved for approval. core team members must be achieved for approval.
How to review a documentation patch
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Before you proceed with reviewing patches, make sure to read carefully the
`Review Guidelines`_ for documentation and `Code Review Guidelines`_. Once
done, follow the steps below to submit a patch review.
#. Go to the `Documentation Program Dashboard`_.
#. Click a patch set.
#. Click a file that was uploaded to view the changes side by side.
#. If you see some inconsistencies or have questions to the patch owner,
double click the line in question for a *Comment* field to appear.
Click *Save* button once you write a draft of your comment.
#. In the *Jenkins check* section, click the Jenkins *checkbuild* gate
link (for the openstack-manuals, it is called
*gate-openstack-manuals-tox-doc-publish-checkbuild*) and review the
built manuals to see how the change will look on the web page. For a new
patch, it takes some time before Jenkins checks appear on the Gerrit
page. You can also `build the patch locally`_ if necessary.
#. Click *Reply* to vote and enter any comments about your review,
then click *Post*.
.. note:: A patch with WorkInProgress (WIP) status needs additional work
before it gets merged. Therefore, you may skip such a patch and
review once it is ready. For more information, see
`Work In Progress`_.
.. seealso:: `Peer Review`_
.. _`build the patch locally`:
How to build an existing patch locally
--------------------------------------
Before proceeding, make sure you have all the necessary
:ref:`tools <setting_up_for_contribution>` installed and
set up for contribution.
To build a patch locally:
#. In terminal, switch to a necessary directory. For example::
cd openstack-manuals
#. Run the below command to create a local branch with the patch in question::
git review -d <nnnn>
where the value of <nnnn> is a Gerrit commit number. For example, 226632
is the commit number of the patch https://review.openstack.org/#/c/226632.
#. Build all the books that are affected by changes in the patch set::
sudo tox -e checkbuild
#. Find the build result in :file:`openstack-manuals/publish-docs/index.html`.
#. Review the source and the output. You are also welcomed to edit and update
the patch:
#. Ensure that your edits adhere to the
:ref:`Writing Style <stg_writing_style>` for OpenStack documentation
and uses standard US English.
#. Once the build and new output are good to commit, run::
git commit -a --amend
#. When the editor opens, update the commit message if necessary. But do
not add information on what your specific patch set changes. A reviewer
can use the Gerrit interface to see the difference between patches.
#. Save the changes if any and exit the editor. If your editor is vi,
use the :command:`:wq` command to save the file and exit vi.
#. Send your patch to the existing review::
git review
#. Leave a comment in Gerrit explaining the reason for your patch set.
.. TODO: make a seealso and add a links to the below pages once converted to
.RST:
- https://wiki.openstack.org/wiki/Documentation/HowTo#Building_Output_Locally
- https://wiki.openstack.org/wiki/Documentation/HowTo#Using_Tox_to_check_builds
.. _`Documentation Program Dashboard`: http://is.gd/openstackdocsreview
.. _`Code Review`: http://docs.openstack.org/infra/manual/developers.html#code-review
.. _`Review Guidelines`: https://wiki.openstack.org/wiki/Documentation/ReviewGuidelines
.. _`Code Review Guidelines`: http://docs.openstack.org/infra/manual/developers.html#code-review
.. _`Peer Review`: http://docs.openstack.org/infra/manual/developers.html#peer-review
.. _`Work In Progress`: http://docs.openstack.org/infra/manual/core.html#work-in-progress