Friday, October 23, 2020

2.5 Code Review

 Code review is a phase in the software development process that helps identify bugs and poor code practices, as well as improve design and overall readability of the code. Its main goal is to find bugs before code is deployed to production, or even to a test server. Code review is a very important part of the development process, but it is often overlooked due to the lack of time or resources.

In the code review phase of software development, at least one person checks the source code of a program.

Reasons for a code review:

  • Identify bugs.

  • Improve code quality.

  • Get familiar with different parts of the project.

  • Learn something new.

Note

There are many tools for formalizing a code review process. From mailing patches (like the Linux kernel repos), to using web applications like GitHub (pull request), GitLab (merge request), Gerrit (changeset), etc.

Identifying bugs is probably the most important reason to do code reviews. The last thing you want is your code to break in production. By reviewing other people's code, you can improve it or learn something new. Either way, by reviewing the code, you expand your knowledge of the project, becoming more competent to work on other parts of the project.

Who should perform the code review? Source code might be reviewed by other software developers, testers, or even people who are experts in the area that the source code covers. In a typical environment, multiple people are assigned as code reviewers. In addition to checking the syntax of the code, their tasks are to verify that the code works and sufficient tests exist, and to improve the code if possible.

When should code reviews be performed? The code review phase should be a part of every software development project, not just in large-scale projects. Reviewing code takes a little more time and effort before delivery, but the code will be more stable and easier to maintain. For the code review to be as efficient as possible, smaller code changes are preferred. With large code changes, bugs are harder to identify in a single code review. Large code changes will also take a great amount of time to be reviewed. Each code reviewer should be able to properly review code in 60 to 90 minutes.

Multiple tools exist, including GitHub and GitLab, to help you during the code review phase. Developers can initiate the code review by using the GitHub pull request feature or the GitLab merge request feature.

Here is a typical feature development or bug fixing workflow:

  • Create a new branch.

  • Commit code changes to the remote repository.

  • Create a pull request.

  • Perform automated tests.

  • Review code.

  • Accept code (merge).

  • Perform additional automated tests.

The first task of any software developer is to create a new branch where code changes will be made. After the code changes are made and enough tests exist, the code changes are pushed to the remote repository. Code changes can consist of one or multiple commits. Commit messages should be descriptive so that other developers and reviewers easily understand the code changes that were made in a particular commit during the code review.

Once a branch that contains a new feature or a bug fix is ready to be merged, a developer creates a new pull request where the developer selects the source (feature or bug fix) branch and destination branch (among other parameters). In a typical software development environment, pull request creation automatically triggers additional checks that are made before the reviewers begin reviewing code:

  • Front-end code tests

  • Back-end code tests

  • Database migration tests

  • Code style test

  • Building a Docker image test

  • Other tests

Running code tests is the most common step after creating a new pull request. If some tests fail, the pull request cannot be merged, so the developer needs to make sure that all tests pass. Multiple tools (such as Circle CI and Jenkins) can be integrated with GitHub for automated test execution.

Another common check when creating a new pull request is database migrations. The pull request might contain database schema changes that can also be verified by running them on multiple test databases.

After all the checks pass, the code can be reviewed. Code reviewers might request additional code changes or simply approve them. After code reviewers approve the changes, they can be merged into the destination branch. Merging pull requests can trigger additional checks or tasks on the destination branch.

Code Review Example

Assume that you need to create a script that parses customer information (customer names and their IP addresses) from a router virtual routing and forwarding (VRF) and interface configuration. You already have a repository on GitHub, and your coworker already put a router configuration to that repository. Your task is to parse relevant customer information from that configuration.

You should create a Python script that parses the following configuration.

ip vrf CUSTOMER_A
  rd 65000:100
!
ip vrf CUSTOMER_B
  rd 65000:101
!
ip vrf CUSTOMER_C
  rd 65000:102
!
interface GigabitEthernet0/0.100
  encapsulation dot1Q 100
  ip vrf forwarding CUSTOMER_A
  ip address 10.50.100.1 255.255.255.0
  no ip redirects
  ip nat outside
  ip virtual-reassembly in
!
interface GigabitEthernet0/0.101
  encapsulation dot1Q 101
  ip vrf forwarding CUSTOMER_B
  ip address 10.30.17.123 255.255.255.240
  no ip redirects
  ip nat outside
  ip virtual-reassembly in
!
interface GigabitEthernet0/0.102
  encapsulation dot1Q 102
  ip vrf forwarding CUSTOMER_C
  ip address 10.11.12.1 255.255.255.0
  no ip redirects
  ip nat outside
  ip virtual-reassembly in
!

In accordance with software development best practices, you create a new branch called parse-ip-addresses in your Git repository. After you finish script development, you push it to the newly created branch on the remote repository. At this stage, the quality assurance team can execute different tests of the script. After testing is complete, the script is ready to be deployed to production. But because code is deployed to production from the master branch, you need to merge the parse-ip-addresses branch to the master branch.

There are different ways of merging the parse-ip-addresses branch to the master branch:

  • Direct merge: Using the git merge command—not a recommended option

  • Merge using pull request: Requires code review—the recommended option

When changes are pushed to a custom created branch on GitHub, GitHub automatically offers you the ability to create a pull request.

When you navigate to your repository on GitHub, you will be offered an option to compare your changes with other branches and create a pull request.

GitHub automatically tells you to create a pull request when a new branch is pushed to a remote repository with a notification and a Compare & pull request button.



Click the Compare & pull request button, which will redirect you to the appropriate page for creating a pull request.

Open a pull request in GitHub.


On the "Open a pull request" page, you have a lot of options. Some are mandatory, and others are optional:

  • Source and destination branch: In your case, you want to merge the parse-ip-addresses (source) branch to the master (destination) branch.

  • Comment: It is recommended to write a comment to notify reviewers about changes done in a pull request.

  • Reviewers: One or more people should review the changes (software developers, quality assurances team, domain experts, and so on).

  • Assignees: One or more people should review or make the changes and perform the merge into the destination branch.

  • Labels (optional): Any custom-made label to which this pull request belongs.

  • Projects (optional): Any project to which this pull request belongs.

  • Milestone (optional): Any milestone to which this pull request belongs.

On the same page, you will also find information about commits you made in the source branch; all the code changes will be presented as well.

After you select all the desired fields, click the Create a pull request button. When a pull request is created, reviewers and assignees are automatically notified via email. GitHub can also be integrated with other systems (for example, Slack), so there are multiple ways to notify reviewers and assignees about new pull requests.

Once a pull request is created, reviewers and assignees must review the code and either approve the changes or request additional changes.

Reviewers and assignees can start with the code review by clicking the Add your review button on a pull request. In a code review, reviewers and assignees not only check the code visually, they can also check out the code locally and test it. Inline comments are a very useful feature, allowing reviewers to attach a comment to a specific line of code.

When a certain reviewer or assignee finishes with the code review, there are multiple options to submit a review:

  • Comment: Submit general feedback (without explicit approval or denial).

  • Approve: Approve changes that are done; the pull request can be merged.

  • Request changes: Deny changes. Comments made by reviewers and assignees must be addressed before pull request can be approved and merged.

If any reviewers or assignees request changes on the pull request, you need to make the changes and then request the code review again. After all comments are addressed, one of assignees will approve the pull request and merge the changes to the destination branch. After the changes are merged into the destination branch, you can delete the source branch.

When the code review is finished, the pull request author is automatically notified about the changes that were done on the pull request.

After a pull request is approved, code changes can be merged into the destination branch.


At this stage, you can also set additional checks, which must pass to be able to merge the pull request. Integration with a continuous integration platform (for example, Circle CI) is often used as an extra check. Only when all the tests on that server pass can a pull request be merged. The actual merge is then usually performed by the project maintainer or owner.

Merging a pull request can be done in multiple ways and can be enabled or disabled for each repository:

  • Create a merge commit (default option): All commits from the source branch are added to the destination branch in a merge commit.

  • Squash and merge: All commits from the source branch are squashed in a single commit and then merged into the destination branch.

  • Rebase and merge: All commits from the source branch are added to the destination branch individually without a merge commit.

The pull request is merged into the destination branch. The source branch can be deleted.


The source branch can be deleted after a pull request is merged. Repository settings allow you to set automatic deletion of the source branch, once a pull request is approved and merged.

Content Review Question

Which GitHub feature is used to initiate a code review?

No comments:

Post a Comment