Code Review — Why and How

MOTIVATION

What Is Code Review?

When a developer finishes working on an issue, another developer reviews the code and considers questions like:

1. Are there any obvious logic errors in the code?
2. Considering the requirements, are all cases fully implemented?
3. Does the new code conform to the existing style guidelines?

Code reviews should integrate with a team’s existing process. Most modern software developer teams are implementing features on a task level. This means code review should be initiated after all the code has been written for the specific task (user story or bug fix). This prevents poor coding decisions from polluting the main line of development.

What Are the Benefits of Code Review?

• Work is decentralized across the team — code reviews help facilitate knowledge sharing across the codebase and the team, meaning anyone from the team should be able to take over any of the Jira ready tickets.
• The team makes better estimates as product knowledge is spread across the team.
• Code reviews enable time off — nobody likes to be the sole point of contact on a piece of code. Likewise, nobody wants to dive into a critical piece of code they didn’t write – especially during a production emergency.
• Code reviews are used to mentor newer engineers — when new members join the team more seasoned engineers onboard the newer members. Code review helps facilitate conversations about the codebase. Often, teams have hidden knowledge within the code that surfaces during code review. Newer members, with fresh eyes, discover gnarly, time-plagued areas of the code base that need a new perspective.

Note: Code review is not just a senior team member reviewing a junior team member’s code. Code review should happen across the team in every direction. Knowledge knows no bounds!

But Code Reviews Take Time!

Sure, they take time. But that time isn't wasted — far from it!
Here are a few ways to optimize it:

• Requiring code review before merging upstream ensures that no code gets in unreviewed, meaning questionable things and potential bugs get caught before they have a chance to make a lasting impact on your application.
• To avoid bottlenecks, especially in big teams, the team shouldn’t wait for all developers to do the code review over a particular feature (except in cases when some critical changes should be made ). The team can agree on an optimal number of developers that have to be included and whose approval is mandatory for merging, each additional reviewer is a bonus.
  As agile suggests, this is one of the things that can be adjusted over time.
• Use peer pressure to your advantage — when developers know their code will be reviewed by a teammate, they make an extra effort to ensure that they design the code in the best possible way so that the review would go smoothly.

PROPOSED SOLUTION

Git is the most commonly used version control system. Git tracks the changes you make to files, so you have a record of what has been done, and that you can revert to specific versions if you ever need to. Git also makes collaboration easier, allowing changes by multiple people all to be merged into one source.

Collaboration between developers

Git has become a standard, as we are able to work collectively with the source code and push our changes back to the team repository as soon as we are done with our work. This does not mean, however, that there are no unique challenges when working in Git. To solve them, a Gitflow branching model was created. Simply put, it is designed in the following way:

The main branch is a highly stable branch that is always production-ready and contains the last release version of the source code in the production.
The develop branch is derived from the main branch and it serves as a branch for the integration of different features planned for an upcoming release. This branch may or may not be as stable as the main branch. It is where developers collaborate and merge feature branches.

Both of these are protected so that code that is not ready isn’t pushed by mistake.

Besides those two primary branches, there are other branches in the workflow:

1. Feature — is derived from the develop branch and it is used to develop features. This branch is merged back to develop branch with the Merge request after a feature is complete.
   Merge requests are created in the Git hosting platform we use. There we add reviewers who can then do the code review — leave comments, questions and suggestions. The author then addresses them by either changing the code or continuing the discussion in the comments. When the reviewer is satisfied, they approve the MR. Open MRs should be checked ASAP when they are opened so we are not blocking the tickets for too long.
2. Release — is also derived from the develop, and it merges back into the develop and the main branches after the completion of a release. It’s created and used when features are completed and finalized for a versioned release. It should contain the version number in the branch name.
   This approach enables the rest of the team to work on features for the next release while one developer can separately work on this release’s stabilization.
   Once this branch is merged to the main branch, we delete the branch and tag the main branch with that version number for tracking purposes.
3. Hotfix — is derived from the main branch and is used to fix a bug in the main branch that was identified after the release. When this is completed, it is merged back to develop and main branches.

We do this because there is a potential problem you might face when branching off from the develop — some developers might have already started work for the upcoming release while you are still in the middle of the current release. Your release would contain the next version of features, which are not finalized and tested, but you only need to provide bug fixes for the current version. Instead of branching off from develop, you can branch off from the main, as that branch contains only the current version of the code in production. This way, branching off from the main branch will not affect the production nor the development version of the product.

GitFlow example graph