RidgeRun Developer Manual - Methodologies - Code Reviews

From RidgeRun Developer Wiki





Previous: Methodologies/Gitflow Index Next: Methodologies/Project Cycles




Introduction

A code review is a a software quality control technique in which a piece of software written by a developer is reviewed by its peers in the search for defects. The term defects is very broad it includes considered "bad practices" and must not be limited to functionality errors, indentation, misspellings, hard to understand logic and code replication - some of the not-so-popular defects that a code review intends to catch.

This page is structured in two main sections. They address the two main actors in a code review: the developer and the reviewers.

General Guidelines for Developers

This section describe the good practices that are expected from a developer that is submitting a change to a code review. This guidelines are important since, if followed, will ease in great manner the code review process, improving the probability of catching errors and improving review speed.

Keep code reviews small
The smaller the change to review, the easier and effective it is. There is better chance to catch errors and the review may be more exhaustive.
Limit your review to one change
Multiple logical changes are hard to review. It's easier for a reviewer to assume that all the changes proposed are meant to achieve the logic change.
No dirty code
Do not submit code you if you know is dirty and will it be returned for trivial stuff. This includes code commented out, wrong indentation, and logic that you know is broken.
Indent your code
Don't waste reviewer's time with code that is not indented. Use an automated tool such as GNU/indent, Astyle or ClangFormat.
Changes are retroactive
Consider all change requests submitted by reviewers as retroactive. This means that if you find the same defect in other parts of your code, it should be fixed even if the reviewer missed it. Similarly, newly written code should adopt this suggestions. As a developer this is part of the growth required to become a better professional.
Code reviews do not replace tests
A code review is mostly meant to capture style and static code defects. They are not replacement for tests of any kind and should not be treated as such. It's very hard to catch a behavioral defects in a code review.
No responsibility avoidance
The code review is an extra layer of quality control. The developer continues to be the owner of the task/user story and must account for it.
Be proud of your code
What a reviewer may comment on a code review is not written in stone. Don't absorb changes you don't agree with. Defend your design decisions if you feel so. A code review is an open discussion and you may know something that the reviewer doesn't. It may be the case that the discussion leads to a conclusion in which the change is not needed.
Be humble
We are all here to learn. Reviewers are going to be strict and provide constructive criticism because they are supposed to. Strict reviewers make better reviews and at the end better code quality. Don't feel bad if you get a lot of corrections!


Previous: Methodologies/Gitflow Index Next: Methodologies/Project Cycles