NOTE: If you are interested in attending OSCON to check out Emma Jane’s talk or the many other cool sessions, click over to the OSCON website where you can use the discount code OS13PROG to get 20% off your registration fee.
The critique is a design school staple. You can find more than a few blog posts from design students about how “the crit” either made them as a designer, or broke their will to live. Substitute “critique” with “code review” and you’re likely to find just as many angst-filled blog posts in the open source community. Or, more likely, you’ll notice a lack of contributors following a particularly harsh text-based transaction.
Somewhere along the way we dropped one of the original meanings of the word “critique”. According to Wikipedia, a critique is “a method of disciplined, systematic analysis of a written or oral discourse”.
A good critique is more than just a positive feedback sandwich that wraps “negative” feedback between two slices of “positive” feedback. An effective critique needs to have three components in place to be successful:
- An agreed-upon framework for the evaluation.
- Reviewer objectivity.
- A creator uncoupled from his or her work.
The Quantitative Review
Quantitative reviews are the easiest to conduct. And, generally, coding standards for a project are quantitative. A quantitative review answers questions such as: Is this code formatted according to coding standards? Does the code cause any performance regressions? Quantitative evaluations can be easily measured. The code being reviewed is either right, or non-compliant. The findings of quantitative reviews are generally hard to argue with. (Ever tried to pick a fight with Jenkins?) Having a quantitative framework in place often means that your project will also be able to free up reviewer time by putting automated tests in place. Which leaves time for a more difficult type of review: the qualitative review.
The Qualitative Review
A framework for a qualitative review is incredibly difficult to create because there is space for subjective thought as it addresses the values of a project. There is room for opinion. Two people could be right at the same time, even though they are not at all in agreement. Qualitative frameworks are so difficult to create that most projects don’t even try; however, an effective qualitative framework helps both the coder and the reviewer to improve their skills. A qualitative framework teaches non-experts what attributes they should be looking for in the code they are reviewing. Questions posed during a qualitative review may include: Does this code implement a pattern we’ve already seen somewhere in the project? Has the code been sufficiently abstracted to make it reusable in other situations?
The Difficulty of Clear Communication
Conducting an evaluation is inherently a “judgy” activity. Good reviewers are able to phrase their feedback so that it ties the work to the stated framework without inserting too much of themself. It also gives space to the creator of the work to respond in an equally neutral manner.
In a perfect world there would always be a perfect connection between what the reviewer meant to say and what the creator heard. Too often it doesn’t happen—especially in our deliciously international FOSS contributor pool. People are often writing (and reading) in their second or third language. Communication is difficult and neutral communication especially so. In fact neutral communication often falls victim to hostility bias—a human trait which causes us to interpret ambiguous communication as hostile.
The Helpful Tips
You can improve the review experience for everyone by following these simple guidelines. Before you begin: Confirm the right type of evaluation is being conducted. Only once the community has agreed that the feature, and broad implementation details are correct for the project, should the code be written and then reviewed. During the review, ensure you follow these simple tips:
- Limit your review to the scope of the work. An effective review is laser-focused on the problem that is being worked on. Create follow-up action items (e.g., tickets) for any out-of-scope work that you think needs additional consideration.
- Make your review specific. Address specific lines, or components in your review. Avoid making general, sweeping statements.
- Make your review actionable. If it’s not immediately obvious how to implement the change you’ve requested, leave it out of your review.
- Deliver your review in a timely manner. The longer you wait to give your review, the more the creator will have to draw from their memory on why certain decisions may have been made. I’ve started scheduling review times with one of my co-workers so that we can go through the work together. This isn’t always possible, or desirable, but it can be effective for small teams.
- As part of your review, be sure to acknowledge the time spent by the coder on the issue. Sometimes a three line change can take hours and hours of work. Take the time to be a human being and acknowledge the time that’s been spent on the issue.
- Be thankful. Especially on open source projects where contributors are generally not *required* to submit anything. And especially at work where co-workers are sometimes forced to work on parts of the project they genuinely hate. Take the time to say “thanks”. It can make all the difference.
If you want to go even further, and learn how to create effective qualitative evaluation frameworks, I encourage you to join me at my OSCON session.
[adrotate banner=”7″]