Build Better App & Team with Code Review

At Ice House, we have a code review process in place to ensure and maintain the quality of our apps. The process took place regularly throughout a development lifecycle, where a group of developers review any code changes during a pull request before it is approved and committed to the codebase and finally tested by the QA team.

Why do Code Reviews?

Code reviews help identify any early defects or coding errors, which serves as an important checkpoints throughout the development lifecycle, because finding bugs gets more expensive over time and even more so when a bug is found during the post-release or in production stage. But over time, when code reviews are done correctly, they are actually doing more than that. Besides the technical and quality benefits, code reviews actually help build better teams as well.

Better Code Quality

By doing frequent code reviews, the following are possible to do throughout development lifecycle in a regularly manner, which helps maintain code quality:

  • Requirement implementation verification: verify that the changes meet requirements of the feature including compliance to non-functional requirements (i.e security & performance requirements), adhere to agreed software architecture pattern (if any), are testable, and within scope boundaries.
  • Maintaining readability and consistency: maintaining code readability and consistency throughout the codebase, compliance to coding conventions and/or specific company style guidelines.
  • Defect and bug finding: collaboratively identify and fix any bugs, logic mistakes, or code vulnerability as early as possible.

Encourage Teamwork & Learning

The following are the benefits of code review to the team which encourage teamwork and learning:

  • Increased sense of mutual responsibility to the codebase: promotes an atmosphere where the entire team is responsible for improving code health of the codebase.
  • Knowledge spreading: provides sharing and learning opportunities between senior and junior developers, including introducing better techniques or approaches to solve a problem. In case of a new developer joining in later stages of development, it also helps getting familiar with the codebase.

Code Review in Practice

Preparing Code for a Review

Preparing for a code review is as important as getting through one for a smoother and faster review process:

  • Understand and clarify requirements before implementation. Avoid any additional code changes or implementation that is not needed now or even in the future.
  • Try to submit code changes in small chunks and as often as possible. Small code changes allow many advantages that we will discuss later.
  • Every code change submission should have unit tests along with it. Make sure it has good coverage test cases i.e. positive, negative and possible edge cases
  • Double check by performing code review on your own code. Make sure the code works and passed unit tests one last time before submitting for a code review
  • Write brief and clear commit messages (as well as writing descriptions during pull/change requests) to help reviewers understand your solution without headaches and better tracking history. For example:
    • Update JJWT implementation on TokenAuthentication class
      Updates implementation parser on TokenAuthentication class since a new JwtParserBuilder interface has been added in JJWT 0.11.2 and is the recommended way of creating an immutable and thread-safe JwtParser instance.
      • For the first line, write a capitalized short summary of what is being done (an imperative sentence with maximum 50 characters or less) and followed by an empty line.
      • For the rest of the body, write a brief description of what’s being done in the code. Always provide enough information on what’s being done, one-liner messages such as “Fix bug A” are too short and do not provide enough useful information for future readers.

Getting through a Code Review

Performing code review can be a daunting task, especially if you’re unsure what to look out for and where to begin during a code review. As a general guideline:

  • Prioritize code review whenever there are no immediate tasks at hand since it may block other developers working on a new feature based on the changes.
  • Take an appropriate amount of time to review,  no more than 400 – 500 lines of code at a time. According to a code review study at Cisco Systems, reviewers became less effective when doing a faster code review with the pace of more than 500 lines of code per hour.
  • Be constructive, highlight and compliment on good practices.

What to Look Out for in a Code Review

  • Purpose and design: one of the most impactful aspects of code review is making sure the changes meet requirements and are designed thoughtfully. Some questions to ask yourself during the review:
    • Do the changes meet all requirements of the feature?
      • Functional requirements fulfilled
      • Compliance to non-functional requirements (i.e security & performance requirements)
      • All changes should be made for a reason and avoid over engineering
    • Do the structure of the code changes make sense?
      • Not too complex, can be understood quickly by the readers. 
      • Easy to change and does not prone for errors when modified by future developers
      • Adherence to software architecture pattern
    • Are there any pieces of the code that should have belonged elsewhere?
    • Are there new dependencies?
      • Check for known defects & security issues
      • Refine dependency configuration if needed (e.g disable feature we don’t actually need, adjustment)
      • Check their usage license
  • Implementation and functionality: take time to review every function implementation in detail. Run automated and functional tests to make sure it is working correctly. Some more questions to ask yourself during the review:
    • How would you have solved this problem? If it’s different from the author, why?
    • Are all inputs validated?
    • Are there any code duplications?
    • Are there any unit, UI or end-to-end tests?
      • Tests should be correct, sensible and useful
      • Ensure sufficient test coverage: positive, negative and edge cases
  • Readability and consistency: make sure that the code is easy to understand and matches the current style of the codebase. Some more questions to ask yourself during the review:
    • Any code conventions or specific company style guidelines that should be followed?
    • Does everything have clear and sensible names?
    • Is the coding style consistent with the codebase?
    • Are documentations updated to match the current changes?
      • Ensure API documentations, database schema, designs and architecture diagram are up to date
    • Are there sufficient code comments and documentations?
      • Explaining why some pieces of code is required if not self explanatory

Small vs. Large Changes

It’s always a good idea to have code changes reviewed frequently and in small sizes. Large code changes tend to overwhelm the reviewer and reviewing becomes less efficient i.e decrease in defect finding, taking too much time, and more stressful experience since a person can only process so much new information at once. In contrast, small code changes will:

  • Have detailed and quick reviews: reviewers will find it easier to find time to do the review and will be more likely to have detailed inspection.
  • Less likely to introduce defects: with fewer changes, it’s easier for the reader to think about the impact of code changes, identifying concerns and finding any defects that may occur.
  • Easier and faster merging: smaller code changes means taking less time to work on it and less likely to cause conflicts when submitting to the codebase.

If for any reason code changes need to be large, the author should provide annotations/comments as detailed as possible prepared during the code review and guide the reviewers for better and quick understanding of the changes. Such changes can also be better reviewed in person first, so that the author can explain their changes at a high-level and guide the review, and reviewers will have a forum to ask initial questions.

Responding to Comments

Understanding code to review can be frustrating which can cause varied comments from reviewers. When responding to those comments, always have in mind:

  • Development goal: the goal of the review is to maintain the quality of the codebase and deliver the highest quality app, which requires effort from all members of the team like yourself and the reviewers.
  • Valuable feedback: no matter how confident you are with your own changes, always consider if the reviewer is providing you with feedback that helps improve the quality of the changes even further.
  • When to refactor or fix the code: when the reviewer does not understand the code immediately, there’s a chance that future readers will also neither. Try to add brief but meaningful comments to your code or consider refactoring/clarifying the code if adding comments doesn’t help.

A Case Study at Google

Code review has been an important and a required part of the software development process in Google since the company’s early beginnings and has been refined over more than a decade.

Early Motivations and Current Expectations

Code review at Google was first introduced to ensure readability and maintainability. Since code is read many more times than it is written, then it must act as a teacher for future developers. Code review was also seen as capable of making sure that more than one developer is familiar with every part of the code and increasing the chances of knowledge staying within the company. Soon enough, there are other benefits became clear and transformed into expectations when doing code reviews at Google:

  • Education: refers to teaching or learning from a code review. Broadly valued and depends on the work relationship between the author and reviewers.
  • Maintaining norm: refers to maintaining organization’s preference for a discretionary choice such as formatting or API usage patterns
  • Gatekeeping: concerns on the establishment and maintenance of boundaries around source code, design choices or other artifacts
  • Accident prevention: refers to the prevention of bugs, defects or other quality related issues.
  • Tracking history: refers to tracking history of code changes and capturing developer interactions (logs) with the code review tool

Code Review Tools & Process

Several of Google’s open source projects, such as Chromium, used an externally available tool-based review, Gerrit, where the code review process is quite straightforward: changes are only merged into the codebase only after explicit approval from reviewers and automated verification that the submitted change does not break the build. For internal software development however, Google has an internally developed code review tool called Critique and additional requirements to pass a code review. One of the notable features of Critique is that it shows automated code analysis results from a static code analysis tool, called Tricorder, as comments along with others from the reviewers. The Tricoder is at least capable of analyzing more than 30 programming languages and helps speed up the code review process by providing suggestions based on automated checks results.
Google has a company wide requirements that must be fulfilled in order to push code changes into the codebase. Most internal software development at Google has a monolithic source repository and accessed via an internal version control system. The codebase is arranged in a tree structure where each directory is explicitly owned by a set of people (owners). Any developer can propose a change to any part of the codebase provided if at least there’s a reviewer that has ownership over the code and a reviewer with the right readability certification for the code language. It’s not necessarily required to have at least two reviewers, most of the time one approval is often enough.

Despite the strict requirements, code review at Google is a lightweight process which mostly due to the smaller changes and quick reviews. Most code changes happen in small chunks, over 35% of code submitted for a review modifies only a single file and about 90% modify fewer than 10 files. The overall median latency for the entire review process is under 4 hours (including waiting for the reviewer to give feedback) and one reviewer is often seen as sufficient.

Code review is broadly seen as being valuable and efficient within Google. Despite challenges, which mostly occur due to the complexity of interactions that occur around the reviews (i.e. misunderstandings and mismatched expectations), most developers at Google are happy and satisfied with it.

What is Readability Certification?

Readability certification is an internal certification that shows a developer at Google understands the company’s coding style guidelines and best practices for a specific language. Every code change must be authored or reviewed by a developer with the right readability certification before being pushed into the codebase or production. For example, if a developer needs to submit Kotlin-based code to the codebase, then it is required to have a reviewer with Kotlin readability certification. To get this kind of certification, a developer is required to submit and get approved his code changes to a team of readability reviewers who will check the code under scrutiny and hold nothing back.

References

  1. Exponential Cost of Fixing Bugs, https://deepsource.io/blog/exponential-cost-of-fixing-bugs/
  2. Google’s Engineering Practices documentation, https://google.github.io/eng-practices
  3. Modern Code Review: A Case Study at Google, https://research.google/pubs/pub47025/
  4. What It Takes to Get Certified to Review Code At Google, https://dev.to/pullrequest/getting-the-certification-to-review-code-at-google-55ng
  5. A Note About Git Commit Messages, https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html
  6. Best Kept Secrets of Peer Code Review, https://smartbear.com/resources/ebooks/best-kept-secrets-of-code-review/
9
Share

More Articles

Loving what you're seeing so far?

It doesn’t have to be a project. Questions or love letters are fine. Drop us a line

Let's talk