October 14, 2013

Finding the Balance of Freedom and Control in Code Review

Community
Helix Swarm

Image: via Flickr

The Balance of Freedom and Control

A lot of Perforce users really like what they see when I show them Swarm. Almost everybody involved in Software Development wants to do some kind of code review, however, there are discussions on how liberal or how strict a review process should be. In every development process the pendulum swings more towards freedom or more towards control.

From the outset Swarm is flexible and doesn’t impose a formal process. While this is appealing to the developer, other stakeholders in the development process may desire a little tighter control. Swarm also supports processes for tighter control and in the documentation you can find guidance on how to configure your Swarm installation so that it enforces reviews for a given Perforce depot location. This makes use of Perforce triggers to ensure that only reviewed changes make it into the Perforce history.

While this is good and appropriate, especially for very sensitive code or ready-to-release software, automated enforcements have the disadvantage of getting in the way just at the very moment you need to get something done quickly - for example, a reviewer isn’t available and you find yourself in the situation of “faking” the approval or going to the administrator requesting the trigger be temporarily disabled. In the worst case you may even bypass versioning altogether. And quite often these tweaks don’t get addressed or reverted once the original problem is out of sight.

Finding a Common Ground

I’m always tempted to propose some sort of compromise, especially when implementing new processes in an organisation. Such a compromise may be seen as rather liberal but has an element to it that helps motivate the stakeholders to adopt some “good behaviour” over the course of the journey. This ideally leads to an outcome where the enforcement becomes obsolete or not harmful if left in place.

There are a number of mechanics useful to accomplish such a compromise. In general they involve the concept of providing the stakeholders with information how something should be and doesn’t “slap” them.

Examples for this would be:

  • Instead of having a trigger that rejects Java code because it doesn’t pass a checkstyle test, it is much more “friendly” to have code formatting or templates in the IDE that do it the right way from the very beginning.
  • When you enforce a specific entry in your changelist description with a trigger you could also assist the developer by creating a custom submit dialog in P4V showing dropdown lists or pre-populations.

And very often something as simple as an auto-generated report that shows areas of improvements will do the trick.

Reporting Changes with Matching Review Data

Perforce together with Swarm has all the information required to create a report of changes that have not been subject to a review or where the review was not approved. These are changes that you might want to look at more closely. You might want to request a review for them. You might eventually even want to back out a changelist completely.

Both Swarm and Perforce provide API’s to automate the generation of the report. Perforce users know that there are many API’s for Perforce, from basic C/C++ to more high level scripting languages. Swarm, being a web service, exposes a RESTful API. Using a Perforce API you can gather all your change information and with the Swarm API all your review information and combining these within one script can create the report. Modern scripting languages make it easy to retrieve and parse the JSON formatted data provided by Swarm.

Coming up: Part 2 of this article will focus on a very simple implementation that makes use of Groovy as the scripting language and the P4Java API to talk to Perforce.