Tag: code reviews

Posted on
by

Code Reviews

Writing code is a tedious process as it involves many conditions. Often the programmers miss out scenarios which can result in bugs (rainy day scenarios). Another common scenario missed out is optimizing the code for space and time complexity (efficiency). The programmers will be busy trying to address the problem at hand and many a times they miss on optimizing.

When we have Peer code reviews, this can be improved. Having few issues with the code is fine but not having code reviews is not acceptable in any software development team.

Static Code Analysis

In simple terms this method is to assess the behavior of the code by performing lexical, syntax analysis as well as other advanced means. Control flow & Data flow analysis are also performed. Findbugs for Java is to a well established example.

Security considerations are one of the major aspects that static code analysis often targets. OWASP wiki entry on static code analysis is an excellent starting point to learn more.

Linting

Some of the basic static code analyzers are called Linters. Though there is no hard and fast rule to call a certain code analyzer as linting tool I think its safe to assume that the coding standard checkers can be called as a linting tool. The term is originated from a static code analyzer called “Lint”.

Code Review tools

The process of code review is quite tedious and as the complexity of the project increases, this gets tougher. There are numerous tools that helps developers to conduct effective code reviews. With the advent of Continuous Integration, its lot more easier to incorporate code review in an automated fashion. Once the automatic code review is done, its essential to report the issues in a easily addressable way.

New generation Project management and Source Control Management Software like Gitlab integrates automation of the review, CI and CD in an innovative manner.

A Gitlab repository which is a CI + CD back end for a Github project.

In addition to the modern all-in-one platforms, there are obviously tools like Reviewboard and others which is focused on the code review alone.

Review Board an excellent, dedicated code review tool.

Phabricator

Meet Phabricator – an excellent project management & source code management platform. The platform has a powerful CLI interface which makes it the swiss army knife of Software Development Process.

Well, I have not heard about it, who uses it ?

Honestly this is the most frequent comment I have heard about Phabricator. So here is a list of users:

  • Wikimedia Foundation – yea, that small website guys!
  • Facebook
  • Asana.com
  • Blender (https://developer.blender.org/)
  • KDE
  • then of-course, yours faithfully!

I rest my case.

In my opinion, Phabricator is the best project management and source hosting platform existing with its unique code review features, CLI tools etc. When it comes to CI and CD, phabricator is not the best and with tools like Gitlab and we just have to mirror Phabricator hosted repository and take care of the CI and CD. Needless to say, Phabricator also supports Subversion and Mercurial. Since they provide a powerful programmable API, it may be even possible to integrate with other code management solutions like Preforce.

Arcanist

This section is about arcanist & is intended for users who are new to it or know about it but haven’t set it up anytime.

Arcanist aka arc is the command line tool which is provided by Phabricator to help with code reviews, merging etc. In a nutshell, we can raise a code review, with pre-defined static code analysis and rules using this powerful tool.


There are four sections of this article:

  • What is it?
  • Which features can we use?
  • Why should we bother?
  • Quick start guide

What is it ?
Arcanist basically works on top of tools like Git, Differential, Linter etc and provides command line interface to them. It is a code review and revision management utility.

Which features can we use ?

  • lint
  • diff
  • land
  • anoid

Why Should we bother ?

Lint: Wouldn’t it be nice if someone could look into your code and point out syntax errors, wrong use of constructs, use of undeclared variable and many more ? It turns out that there are tools which do this exact work and are referred to as Lint. It also simplifies code review process for the reviewer as well as the author

Diff: Working on a project and not sure whether the changes made are ready to be pushed? This is where diff comes into play. If using Git, arc diff sends all commits in a range for review. By default, this range is 

git merge-base origin/master HEAD..HEAD

Land: If the review raised through gets accepted, then we use arc land to publish the changes.

Quick Start Guide

Supported on: Linux, Mac OS X, Windows, FreeBSD & there is a quick way to setup anywhere with NixOS.

The one liner install on macOS and Linuxes using the awesome Nix package manager :

sudo mkdir /nix ; sudo chown -R  $LOGNAME /nix; bash <(curl https://nixos.org/nix/install) ; nix-shell -p arcanist

The manual, old school, Installation:

  1. mkdir ~/phabricator && cd ~/phabricator
  2. git clone https://github.com/phacility/libphutil.git
  3. git clone https://github.com/phacility/arcanist.git
  4. Add ~/phabricator/arcanist/bin to your PATH environment variable

Example:
export PATH=”/home/user/phabricator/arcanist/bin:$PATH”

  1. Try typing ‘arc’, if it shows usage exception, then we are good so far.
  2. To set up tab completion add the following to you PATH environment variable source /path/to/arcanist/resources/shell/bash-completion
    Example:-
    source /home/user/phabricator/arcanist/resources/shell/bash-completion

Configuring arc for a project :

This section assumes that you have a Phabricator installation at https://phabricator.steem.io

  1. Goto project directory
  2. Create a file with name ‘.arcconfig’ (without quotes)
  3. Paste the following in the file.
    {"phabricator.uri" : "https://phabricator.steem.io"}
  4. Run
    arc install-certificate and follow the instructions.

The commands could be used in this sequence:


arc lint –> arc diff –> arc land

arc lint

Setting up lint :


1. Create a file with name ‘.arclint’ in project directory
2. Detailed documentation for setting up .arclint can be found here.

Example of .arclint file:-
{
“linters”: {
“lint”: {
“type”: “pep8”,
“include”: “(\.py$)”
}
}
}
“lint” –> this is a custom name given by you, it doesn’t affect anything.
“type” –> to specify the linter we would like to use.
“include” –> regex for the format of files to lint.
“exclude” –> can be used to exclude files matching include tag in specific directories.

arc diff

  • First time while using this command, it will ask for access token which can be obtained by following the instructions.
  • Specify Test plan, reviewers and proceed.
  • A review request can be updated any number of times before it has been reviewed or separate reviews can be raised using
    arc diff –create arc land
    Once a review gets accepted, the changes can be published using this command. It is the last step in the standard Differential pre-publish code review workflow. arc anoid

Extended read: arc tasks, arc browse.

Encouragement

The Phabricator command line tooling sounds little weird when we first read the documentation. But once you setup, which is very fast if you follow the documentation as it is, its very powerful. The tools are written in PHP7.X and don’t be concerned, its blazing fast. I have been extensively using it, even to review documents. ie, instead of Google Doc, Tracking via Mircorsoft word’s tracking etc.

A sample document under review.

Summary

As already mentioned, test-driven, peer-reviewed code is the only acceptable way to develop software. Code review tools like Phabricator’s inbuilt tools are highly recommended irrespective of the tooling, language that a developer is using.