Zaqar/Your first review
Reviewing and 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 Zaqar fixes.
- Select a fix from the list to review.
- Download the patch to your local repository and test it.
git review -d [review-id]
The review-id is located the number in the URL (check the screenshot for more details).
git review -d 92979
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 the review process easier. As you compose your comments, please keep in mind these guidelines.
- 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, and when you are done just click "Publish Comments".
- 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! Also, try to select an easy patch for your first review. That will help you to gain some confidence.
- 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.
- 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 it the review comment and score with -1 if you think that it's that important.
- Code style feedback. Do you think that the code structure could be improved? Keep the DRY, YAGNI and KISS principles in mind.
- Grammar/orthography feedback. Many of our contributors are not native English speakers, so its common to find some of this errors. Git Commit messages are as important as code, so you should review it too. Check GitCommitMessages for more information on how a Git Commit message should be written.