What is a code review?
Code review, also known as peer code review, is a process in which a development team reviews the application source code to check it for correctness against a requirement set, spot potential bugs, improve quality and share knowledge among the team.
In a code review, there are at least two roles always present in a code review including the:
- Author, who is the person responsible for creating the code that will be reviewed by the team
- Reviewer, who is the person responsible for examining/reviewing the code and reporting the findings to the author and project team
Here at Coderus, the primary way we conduct code review is through a pull request. Each time a developer wishes to make changes to an application whether it’s for a new feature or to fix a bug, he or she must submit changes for approval to the rest of the project team. This pull request compares the current state of an application source code with the changes a developer would like to make. A series of automated checks are then performed before code is manually reviewed by other developers.
The Code Review Process At Coderus
Here at Coderus, all changes to the code base must go through a pull request in order to be merged into the application. Before code can be merged the automated checks performed by our continuous integration (CI) system, typically Jenkins must pass and have been approved by at least two members of the project team.
These automated checks and their strictness vary depending on the platform and project. For a typical Android project, they’re performed on each commit and pushed to a remote repository, ensuring that the code base compiles and all automated tests pass. Afterwards, a series of analysis tools are run including Ktlint, Detekt and Android Lint – each of which has a different focus. Any problem identified by these tools causes a failure and an alert that is then posted to the Slack channel for the project so that the relevant developer is made aware and can resolve the issue.
Ktlint enforces that the code style matches the official Android Kotlin Style Guide. Maintained by Pinterest, the tool acts as a neutral party to dictate formatting and preventing Bikesheeding, where a development team spends time discussing subjective opinions on formatting such as the positioning of spacing, brackets and line breaking through the code base. Having a consistent codebase makes it easier to read and navigate. Think about how confusing a word document with multiple font types within a paragraph would be to read – the same applies to source code. By automating the process, reviewers can focus on the content of the changes.
Detekt is a static code analysis tool which checks for common errors and code smells. You can see the full list of checks performed on the ‘Rules sets’ page of the documentation. In particular, the rules focusing on complexity such as class and method size, as well as those limiting nested block depth or certain statements, are invaluable. Complicated code is more prone to errors and is harder to maintain and extend. These rules allow complexity to be built using smaller modular components, which are less prone to errors.
Android Lint developed by Google checks for structural problems and Android specific problems with a focus on reliability and efficiency and covers a range of issues such as unnecessary namespaces in XML files and increasing processing time to usage of deprecated APIs.
Once all automated checks pass, the code is then reviewed by other members of the team who leave comments and ask questions. This code review process promotes a discussion about the changes examining a number of different aspects and ensures that the task requirements have been met, potential bugs have been spotted and the code quality can be maintained and improved. Any changes the team agrees to make should be made immediately before the pull request is merged, although new tasks can be created and added to the backlog if the suggestions are outside the scope of the current task. Each project repository includes a contributing.md file, which specifies pull request requirements and this integrates with BitBucket to allow tasks to easily be created when requirements are not met.
Typically a pull request has 2-5 reviewers. Once it has been approved by 2 other members of the project team, one of which must be a senior developer, the changes can be merged into the main development branch of the codebase. Although only two approvals are required, wherever possible the Android team has a culture of waiting for everyone working on the project. This is because anyone can offer valuable insight from a new perspective and contribute to code quality as well as allowing the highest level of knowledge sharing.
When pull requests are created, Slack integrations notify everyone that they have a new pull request to review. Within the next few hours, that pull request should be reviewed to allow the author to make any final changes before merging. While a developer waits for their team to review the changes, he or she starts a new task in a different area of the codebase to prevent overlap. No individual should have more than 4 pull requests open at any one time as pull requests left open for long periods risk merge conflicts or overlapping with other tasks. If a pull request is blocking a developer from getting on with his or her work then he or she is not afraid to follow up with team members to get the approvals they need.
Benefits of a Code Review
This code review process benefits the organisation, developers and the codebase. Essentially, code reviews:
- Allow developers to detect and fix bugs earlier, saving costs and reducing the defect rate
- Ensure that there is consistent coding standards and compliance across all code everyone produces
- Facilitate knowledge sharing and skill development
- Improve code readability and quality by peer pressure
- & Means better estimates and planning of projects
Learn more about some of the key benefits of a code review in our expanded review of a few, below.
Reduces Costs & Defect Rate
By having the project team review code at the point of change, the risk that requirements are missing or misunderstood is significantly reduced. It also means that development teams can identify bugs and resolve them early before they reach the QA team, client or even users and reduce costs.
By promoting a discussion, sharing ideas and suggestions, code can be refined to improve quality. Small improvements to names or suggesting helper functions can improve readability. Discussions around structure and architecture help to clearly define roles and responsibilities for components. Better code quality allows the development team to build new features faster with a lower defect rate.
Facilitates Knowledge Sharing & Skill Development
Code review is also a process of knowledge sharing among team members and in this, everyone is exposed to and contributes to different parts of the codebase, helping to build a shared sense of ownership.
By reviewing code for features or parts of the codebase, which a developer may not have worked on themselves, everyone gains a more complete picture of the codebase and reduces the risk of valuable knowledge being solely held by a specific developer. Anyone can extend parts of the codebase without having to defer to the original developer for an explanation.
Code review also helps to share knowledge of platform or language APIs, thus raising the skill level of the team by introducing developers to frameworks they may not have had the opportunity to work with. Shortly after the announcement in 2018 that Kotlin would become an officially supported language for Android development, Coderus made the switch for all new projects. The strong code review process within the team helps to make that transition easier. The concept applies to the multitude of new APIs and frameworks added to the language and platform year after year.
Code Review Principles
Every time a developer submits a pull request there are a few things they should keep in mind. Checkout on our recent articles on commit guidelines. Google also has a great article on writing good descriptions, while lots of others cover best practices.
Perhaps one of the most important things that we ensure every developer remembers is “You are not your code”. Regardless of whether members of our team are a senior or junior developer, there are always opportunities for improvement and we all make mistakes sometimes. Part of working with a team is having people around you who can help you grow and learn. Changes we might suggest various members work on are not a reflection of each person as an individual, but the way each of our developers responds to those suggestions is. As an example, when reviewers leave comments, they could be directed at the code and not the author. We also avoid ‘You’, for example ‘You’re making multiple API calls here when one will do” in favour of “This code makes multiple API calls when one will do”. There are some great articles on being a good reviewer.
Principles for code review is an expansive topic, but here are a few to keep in mind as both an author and a reviewer.
- Every pull request should have a clearly defined scope without being too large to ensure that changes can be effectively reviewed.
- Can any abstractions be made to improve structure and maintainability?
- Have platform or language conventions been followed?
- Do the changes maintain clear separation between layers. E.g Is business logic separate from the presentation?
- Is the code easy to read and understand? If you’re having problems following the logic, what changes could be made to improve that?
Are your products being built with a solid code review process?
A good code review process helps to deliver projects on time and in budget ensuring the requirements are met and lowering the defect rate. It improves code quality making software easier to maintain and extend, as well as being a great platform for knowledge sharing to raise the skill level of a development team.
If you are looking to partner with a mobile app or software development company with a strong code review process and high standards, get in touch.
