Code Reviews
Code reviews are great in the Software Engineering world, it is a great way to maintain consistency and quality of codebase based on some standards defined by the team, company or industry with some great free benefits like Knowledge Sharing. Code reviews have evolved in the past few decades. It started mainly from human review and now machines help humans in many ways. Human reviewers act as a gatekeeper for the code alongside automated static analysis tools that help both author and reviewer to save a lot of time. This way the focus remains on core things like code correctness and comprehension which you will see in the next section.
As we know, a typical software is written for multiple years, (at big companies even for decades), this requires some sought of standard to maintain and code review is so far the ideal solution. Code review practice comes at the very end of the development cycle right before the PR is created to be merged to the trunk branch.
Benefits
Code Review has several benefits, some of them are:
Knowledge Sharing
When you share, you learn, it is a great opportunity to know what others are doing and how they are doing. Understanding new ways gives an exponential knowledge gain. Learn more about best Knowledge Sharing Practices.
Collaboration
Great way to find new folks, this works well when code reviews are done not within the team but by external folks as well. This helps getting to know a lot of new faces and understanding their point of view.
Code Quality
The main focus of code review is to have code quality, checking the correctness, having it consistent and comprehensive, reviewing code is itself a skill and to be good at that making sure you know how to work well in the teams as written here.
Code Correctness: Focuses on the code being logically and functionally correct and will not potentially lead to any bugs. Having another set of human or machine eye is always beneficial.
Code Consistency: Focuses on the standards as directed by the style guide, mostly handled via static analysis tool. A consistent codebase is easier to understand and contribute.
Code Comprehension: Focuses on how easy it is to understand the code. Remember code is read more than it's written, so it should always be written for the future readers. Having code comments, unit tests and documentation helps.
Some other benefits are ownership/accountability and historical record of reviews.
Best Practices
Be professional and polite
Attacking someone or judging that someone is attacking you on a personal level is very unhealthy for the team. Its all about being kind and polite, this comes when there is judgement involved, example like design document or code reviews. The opinions and suggestions should always be written politely so it does not offend someone, example can be, ‘this is wrong, fix it’ vs `I think it can be improved by doing XYZ, can you look into it please?`
Keep changes small
It is always better to have small changes in PR, it is easier and quicker to review plus avoids the possibility of unknown test case failures. Small changes can be one file involving 10 lines or 10 files involving 1 line.
Meaningful commit messages and description
Having a clear commit message and PR description is always promoted in productive teams, it again helps to understand changes quickly and moves things quicker, like the feedback loop.
Treating Testing, Documentation and Code as one
One PR that adds or changes a functionality should always have the associated updates to tests and documentation, they should be treated as one.
Less the reviewers, more the velocity
Depending on more reviewers decreases velocity, it's better to only have one or two reviewers depending on type of PRs, mostly one reviewer is enough unless the change touches a broader set of code especially owned by other teams.
Save reviewers time by automating
Automating always helps, static analysers, unit tests are ways to help reviewers, just like unit tests are the first users of the codebase static analyzers are the first reviewers of the codebase, most issues related to quality and consistency can be caught by the automated tools.
Types of Code Reviews
Code changes can belong to many different categories so does the review, here is the list of them:
New Codebase
This mostly happens in startups where new repositories are often created either from scratch, this review involves many files with a lot of configuration and setup files. This can be improved by having a templating tool to initialize a standard repository, avoiding duplicate work and also making it quicker to review.
Bug fixes and rollbacks
Production level issues like bugs that require either a quick fix or a complete rollback, in case of fixing the bug with new PR, the review is quicker while in case of rollback its a more thorough review done by someone with knowledge of that system.'
Behavioural changes and improvements
Core changes that change the behaviour by adding a function or updating the function, these situations should be well tested via unit tests, although a reviewer would still be required to ensure code correctness and comprehension. While standards and consistency are typically caught by static analysers.
Large scale changes
These changes are related to refactoring like updating the name of class, function, file, depreciation or standardising the codebase. These changes do not involve any functional change, so unit tests are supposed to pass 100%. It can be performed by automated tools, local IDEs work well on a smaller scale while custom solutions might be needed depending on scale, Google has a dedicated team that performs this type of changes with custom tools and services. It requires a high level review because of too many files, and engineers have to trust the automated tools.
Big Companies vs Startups
Coding standards vary across companies, and there is no one perfect way of doing that, it all depends on team size, company goals and scale.
Big companies have different and strict standards as thousands of engineers contribute to the codebase. Code quality is top priority.
Startups are little flexible, they prefer development velocity over strict rules, no rules might work with a single person team, but when team scales there should be a set of rules set to make sure code is good enough to be merged. Startups with too strict rules would be an overkill and hinder the development speed and frustrate developers making it an unhealthy culture.
This article focused on one of the most important pieces of Software Engineering. Code Reviews are great and should be followed to some extent. The article is curated from the book Software Engineering at Google, which has a dedicated chapter on it with a lot more details.