Jump to: navigation, search

Difference between revisions of "Your first review on Zaqar"

m (Step by step)
Line 17: Line 17:
 
* Select a fix from the list to review.
 
* Select a fix from the list to review.
 
* Download the patch to your local repository
 
* Download the patch to your local repository
 +
  
 
<pre><nowiki>
 
<pre><nowiki>

Revision as of 15:35, 11 May 2014

Reviewing ang being reviewed

The review stage is a very important part in the development process. There are many reasons to say this:

  • Getting other developers feedback minimizes the risk of adding regressions to the code base and ensures the quality of the code being merged.
  • It helps to build community. Everyone appreciate when their code is reviewed, and of course they will review your code when they have their chance.
  • It also helps developers to improve, since they are always learning from other's points of view.
  • Its a great practice to get familiar with the code.


Everyone is encouraged to review code! It's not necessary that you know every detail of the code base, you only have to understand what the code related to the fix does.

Step by step

  • Go to review.openstack.org and filter by Open Marconi fixes.
  • Select a fix from the list to review.
  • Download the patch to your local repository


git review -d [review-id]

The review-id is located the number in the URL (Check the screenshot for more details).


Example

git review -d 92979


Download the patch using the Review ID in the address bar.

This will create a branch with the author name and will allow you to test the patch in your local environment.

  • Inspect the code. Is important that you consider all the good programming practices you know here. Take a look to the checklist for some ideas.
  • If during the inspection you see a specific line you would like to bring up to discussion in the final review, leave your inline comment. This will make easier the review process.
  • Hit the review button in the web UI.
  • Time to score!
    • When to score with -1? If you think that there are things that should be fixed, then voting -1 is ok. We have to be careful with this since we don't want to 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.
    • When to score with 0? If you are the author of the fix and you want 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).

    • When to score with +1? If the fix works and you think that the code looks good, upvoting is your choice.
  • Remember to leave any comment you think is important in the comment form.

Checklist

  • Anyone who is signed in to review.openstack.org can review a patch.
  • If you already submitted a patch, take a moment to think about the reviews you received. Which were the most useful comments? Consider the feedback that

helped you the most in the past and try to do the same for the fix you are reviewing. Helpful review comments will be appreciated by the developer that is submitting the fix.

  • No idea on how to start? Take a moment to check different reviews for patches in review.openstack.org. Don't forget about inline comments!
  • Code location feedback. Do you think that some code should be better located in another place within the file, or maybe in another file? If so, suggest it the

review comment and qualify with -1 if you think that it's that important.

  • Code style feedback. Remember about DRY, YAGNI and KISS.