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 Software code Audit 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.