openstack/deb-zaqar
Code Issues Proposed changes
f35742115f
deb-zaqar/doc/source/first_review.rst
Eva Balycheva b53ff5d12c Refactoring of docs during Mitaka cycle 
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
2016-01-07 17:05:09 +03:00

4.8 KiB
Raw Blame History

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:

$ git review -d [review-id]

The {review-id} is the number in the URL (check the screenshot for more details).

Example:

$ git review -d 92979

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 reviewer_guide for Zaqar inline comments.
  • Keep in mind the reviewer_guide and be respectful when leaving feedback.
  • Hit the 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 Publish Comments.

For more details on how to do a review, check out the Gerrit Workflow Review section document.