b53ff5d12c
This patch improves Zaqar documentation and fixes currently existing bugs. Bugs this patch currently addresses and solutions: Short names for documentation locations used in this commit message: GitRepo - https://github.com/openstack/zaqar/ Contributor Docs - http://docs.openstack.org/developer/zaqar/ Wiki - https://wiki.openstack.org/wiki/Zaqar/ 1. DRY violation and spreaded information for contributors bug. The information for Zaqar contributors is spreaded/duplicated across GitRepo, Contributor Docs and Wiki. Examples of DRY violation are these three articles: https://wiki.openstack.org/wiki/Zaqar/Give_Zaqar_a_try, https://github.com/openstack/zaqar/blob/master/README.rst, http://docs.openstack.org/developer/zaqar/development-environment.html Example of spreaded information is: "zaqar/tests/functional/README.rst" Normally the contributor want to see the information from this file in "doc/source/running_tests.rst". Solution: move useful missing information for contributors from Wiki and GitRepo to Contributor Docs, then replace all contributor information in Wiki and GitRepo with links to Contributor Docs. 2. Outdated information, missing new information and broken links bug. Example is "test_suite.rst": a. It still states that Zaqar test suite lives in two directories - "tests" and "zaqar/tests", but now it's not true. b. Doesn't contain information about how test invocation is organized, what really happens when "tox -e py27" command executes. Solution: replace outdated information with new, fix broken links if possible, add useful missing information. 3. Style and formatting bugs. The reference is http://docs.openstack.org/contributor-guide/. Many documents in Contributor Docs have wrong line wrapping - some lines are wrapped too short and some are wrapped too long. Lines must wrap at 79 characters, exceptions are code and links. Example is "first_review.rst" which lines are not wrapped at all. Enumerated lists must be written using "#. " syntax. Example with wrong enumerated list is "development.environment.rst". Some inline elements must be styled according to: http://docs.openstack.org/contributor-guide/rst-conv/inline-markups.html Example with wrong styling of inline elements is "development.environment.rst" where many paths and file names are not marked with `` (double backticks). By default code inserts are implicitly styled with python syntax. There are many places in Contributor Docs where console (bash) code is wrongly styled with python syntax. Also there are some failed attempts to apply a formatting in Contributor Docs. For example there is a broken list in "first_review.rst" at line 52. Solution: fix broken formatting, apply proper style where it is needed. Some of these bugs fixes closes few bug reports from: https://etherpad.openstack.org/p/zaqar-mitaka-docs Change-Id: Id668684248bdee03eb43b537dc2c6bb2a68ed23d
116 lines
4.8 KiB
ReStructuredText
116 lines
4.8 KiB
ReStructuredText
..
|
|
Licensed under the Apache License, Version 2.0 (the "License"); you may
|
|
not use this file except in compliance with the License. You may obtain
|
|
a copy of the License at
|
|
|
|
http://www.apache.org/licenses/LICENSE-2.0
|
|
|
|
Unless required by applicable law or agreed to in writing, software
|
|
distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
|
WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
|
License for the specific language governing permissions and limitations
|
|
under the License.
|
|
|
|
=================
|
|
Your first review
|
|
=================
|
|
|
|
The review stage is a very important part in the development process. Following
|
|
are some of the reasons this stage is important:
|
|
|
|
* Getting other developers feedback minimizes the risk of adding
|
|
regressions to the code base and ensures the quality of the code being
|
|
merged.
|
|
* Building the community encourages everyone to review code. Everyone
|
|
appreciates having their code reviewed.
|
|
* Since developers are always learning from being exposed to the points of view
|
|
of others, reviews help developers to improve their coding skills.
|
|
* Providing a review is a great way to become familiar with the code.
|
|
|
|
Everyone is encourages to review code. You don't need to know every detail of
|
|
the code base. You need to understand only what the code related to the fix
|
|
does.
|
|
|
|
Step by step
|
|
------------
|
|
|
|
Go to ``review.openstack.org`` and filter by `Open Zaqar fixes`_. Select a fix
|
|
from the list to review. Try to select an easy patch for your first review.
|
|
That will help you to gain some confidence. Download the patch to your local
|
|
repository and test it:
|
|
|
|
.. code-block:: console
|
|
|
|
$ git review -d [review-id]
|
|
|
|
The :samp:`{review-id}` is the number in the URL (check the screenshot for more
|
|
details).
|
|
|
|
Example:
|
|
|
|
.. code-block:: console
|
|
|
|
$ git review -d 92979
|
|
|
|
.. image:: images/zaqar_review_id.png
|
|
:alt: Zaqar review id
|
|
|
|
This git command creates a branch with the author's name and enables you to
|
|
test the patch in your local environment.
|
|
|
|
* Inspect the code. Use all of the best programming practices you know as you
|
|
review the code.
|
|
* Give code location feedback.
|
|
Do you consider that some code should be better located in another place
|
|
within the file, or maybe in another file? If so, suggest this in the
|
|
review comment and score with a ``-1`` if you think that it's that
|
|
important.
|
|
* Give code-style feedback.
|
|
Do you think that the code structure could be improved? Keep the DRY,
|
|
YAGNI and KISS principles in mind.
|
|
* Give grammar and orthography feedback. Many of our contributors are not
|
|
native English speakers, so it is common to find some errors of this type.
|
|
* Make sure that:
|
|
|
|
* The commit message is formatted appropriately.
|
|
Check `Git Commit Messages`_ for more information on how you should
|
|
write a git commit message.
|
|
* The coding style matches guidelines given in ``HACKING.rst``.
|
|
* The patch is not too big.
|
|
You might need to split some patches to improve cohesion and/or reduce
|
|
size.
|
|
* The patch does what the commit message promises.
|
|
* Unit and functional tests are included and/or updated.
|
|
* If during the inspection you see a specific line you would like to bring up
|
|
to discussion in the final review, leave feedback as an inline comment in
|
|
Gerrit. This will make the review process easier. You can also use
|
|
prefixes described in :doc:`reviewer_guide` for Zaqar inline comments.
|
|
* Keep in mind the :doc:`reviewer_guide` and be respectful when leaving
|
|
feedback.
|
|
* Hit the :guilabel:`Review` button in the web UI to publish your comments
|
|
and assign a score.
|
|
* Things to consider when leaving a score:
|
|
|
|
* You can score with a ``-1`` if you think that there are things to fix. We
|
|
have to be careful to not stall the cycle just because a few nits, so
|
|
downvoting also depends on the current stage of the development cycle
|
|
and the severity of the flaw you see.
|
|
* You can score with a "0" if you are the author of the fix and you want to
|
|
respond to the reviewers comments, or if you are a reviewer and you want
|
|
to point out some reminder for future developing (e.g. the deadline is
|
|
the next day and the fix needs to be merged, but you want something to be
|
|
improved).
|
|
* You can score with ``+1`` if the fix works and you think that the code
|
|
looks good, upvoting is your choice.
|
|
* Remember to leave any comment that you think is important in the comment
|
|
form. When you are done, click :guilabel:`Publish Comments`.
|
|
|
|
For more details on how to do a review, check out the `Gerrit Workflow
|
|
Review section`_ document.
|
|
|
|
.. _`Open Zaqar fixes`: https://review.openstack.org/#/q/status:open+zaqar,n,z
|
|
.. _`Git Commit Messages`: https://wiki.openstack.org/wiki/GitCommitMessages
|
|
.. _`Gerrit Workflow Review section`: http://docs.openstack.org/infra/manual/developers.html#code-review
|
|
|
|
|