Contrary to what many developers believe, the programming community did not invent code review. It appears in our culture much before, mainly as a way of censorship.
In the editorial world, there are pieces of evidence of peer review existence since 1731. Modern software development gave it a more specific name: code review. Code Review is the process throughout which your code gets assessed by one or more people. A thorough reviewer usually looks for inconsistencies, errors, potential bugs, problems in design and architecture, and issues of performance and security.
Although not all software companies implement this practice, it is present in a wide variety of them. Waterfall, corporate, and mission critic projects introduced code review much earlier to it become cool. It is not an exclusively agile thing.
When eXtreme Programming got more adopters, developers began to work in pairs. This practice, known as Pair Programming, is a kind of peer review. One programmer types the code, while the other one reviews it. Intermittently, they switch roles.
In my opinion, agile was not solely responsible for peer review propagation in the software industry. The Open-Source community has an essential role in its popularization and improvement. Back in history, patch files used to be attached to emails and sent to all contributors, so they could download, read, run, and review the changed code.
What’s the value of Code Review?
Nowadays, it’s easy and straightforward to create a repository and have your code reviewed by your team through a Merge/Pull Request. New tools, features, and products allowed the increase of productivity by increasing collaboration. Collaboration is the primary value of code review.
Here’s a list of some other benefits, I want to highlight:
- Propagation of knowledge: it’s a moment to teach and also learn. So, share good practices, business rules, framework specifics, alternative solutions, and design patterns, for instance.
- Bugs mitigation: someone else’s eyes on the code may reveal blind spots, not considered scenarios, and even typos.
- Double-check of the functionality: does the code matches both technical and business specification?
- Security and Performance issues: experienced developers can spot common flaws just by seeing the code. A thorough code review can avoid bugs, security, and performance issues.
- Code Quality Assurance: each reviewer must look for a code easy to read, understand, and maintain. It’s part of the reviewer’s job to communicate simplifications and ask for clarifications on messy code. Communication is key to a success code review.
- Team Ownership: when the process of code review is spread out to the whole team, it is incredible how that “this is my code” culture gives place to “this is our code.” Knowledge doesn’t belong to a single person anymore. When your team feels comfortable reviewing everyone’s code, they don’t create silos.
How to achieve code review benefits?
To achieve these benefits, I recommend two things. The first one is a code style guide. In this document, you and your team must include style standards for the code. Discussions, like whether to use double or single quotes are the best, are as useless as what is the best editor.
Each community and programming language has its standard rules. You can adopt an existing one instead of creating a specific to your company. It saves a lot of time and effort in both creating and maintaining the guide.
Having a code style guide avoids those unnecessary discussions, and you can focus on the crucial ones. These issues include architectural design, maintainability, security, and other aspects of quality code.
Of course, writing a style guide is not enough. Your team must follow it. An excellent way to enforce this practice is by running linters. They look for unconformities and tell developers where is the problem, so they can quickly fix them. All you have to do is to configure your linters to match the style guide.
You also can take advantage of tools like SourceLevel. Our tool runs linters in a variety of programming languages and comments straight into your Pull Request. It automates the tedious part of code review.
Secondly, I recommend you to write a review guideline. Some companies prefer having a checklist. However, if you are bumping a version of a dependency library, would you follow the same checklist of a new feature pull request?
A checklist implies checking item by item. Soon this process becomes tedious, mainly if lots of items don’t apply to the proposed change. That’s why I prefer listing all essential checks and make a guide from them.
Younger developers tend to consult the document more often, whereas experienced developers already have the list in them. However, senior or not, it is crucial for your team to revisit the guideline from time to time. It’s perfect for refreshing memory. Also, it’s an excellent opportunity to add or remove items. The content and discussions may concentrate on a private or public repository as SourceLevel does. Continuously maintenance of guidelines leads to your company’s way of performing code reviews.
How to properly review a pull request?
Each business, programming language, and repository require specific checks. There is not a universal list, but I organized a very embracing one you can use as a start point.
As your team overcomes project challenges, learned lessons turn into new items on your list. The same applies to existing items. Remove them if they don’t make sense anymore.
When opening a Pull Request
When opening a Pull Request, you want to be concise, straightforward, and clear about why, how, and what are the changes. To achieve it, I’d suggest you to:
- Add some business context, so your team can understand why this code is being added, changed, or removed.
- If your pull request is a WIP (or draft), add some checkboxes or list with the missing tasks.
- Enrich your description with visual content, like gifs, screenshots, or diagrams. Images and videos accelerate the understanding of the change.
- Defend any arbitrary decision before the discussion to ignite. For instance, if changes are not following a convention, tell your team why you opted by this solution before they point it out. It focuses on the comments and avoids overwhelmed communication.
When reviewing a Pull Request
As I said, there is no such thing as a universal or definitive list. Below I present my suggestion of a code review guideline. If you are an experienced developer, you probably miss some items. The primary intention is to guide those beginning with code review. If you don’t know what to look in a review, this list is a great start.
- Does the code work? Check whether function and logic are correct.
- Are functions, methods, and variables adequately named?
- Are names semantic and meaningful for your business?
- Is it well tested?
- Are there unit tests and they have good quality? Do they test the functionality?
- Do tests reach acceptable coverage for your project?
- Is the code clear and easy to understand?
- Does it meet the team’s guidelines and style guides?
- Is there any duplicated code? Remember that duplication is far cheaper than the wrong abstraction.
- Is there any commented or unnecessary code?
- Does the code take the most out of frameworks and language? Is there any custom implementation of native or already-existing functions?
- Are there code smells, grey areas or bug-prone?
- Is documentation on functions, methods, classes, contexts, and behaviors adequate?
- Is the code as modular as possible?
- Are the critical spots adequately logged?
- Does the code consider failures? Is it just considering the happy path?
- Are there better or simpler solutions?
- Is there any performance issue?
- Are input data sanitized?
- Is there any SQL Injection point?
- Is sensitive information being encoded or encrypted?
Code Review is not just cool; it’s a consolidated practice of the software industry. Many companies already adopted this practice. Open-Source communities heavily influenced and improved tools.
Pull or Merge Requests are nowadays widely known. In this blog post, I presented its value and two suggestions to ignite this practice in your company: having a code style guide and review guideline.
For a code style guide, I strongly suggest your team to adopt an existing one. Depending on your programming language, there may be more than one option. Look for alternatives, discuss internally, and chose one. If possible, automate with linters so you can focus on what matters.
Conversation in pull requests must be something productive. So, I also listed some items for you to start reviewing code more efficiently. This list is not definitive nor complete. It’s up for you and your team to maintain it. Our guidelines repository is public and may inspire you.
Add your repository to SourceLevel to automate code review and get all of its benefits. It’s free for 14 days. Sign up via Github, no credit card required.