Code review etiquette is an interesting subject.
A few months back an article was published to CSS-Tricks titled “Code Review Etiquette”. It outlines some general pointers for reviewers to keep in mind when doing code reviews.
In this post, I’ll make some etiquette suggestions that apply to a more specific part of the code review process…addressing code review comments.
We’ll look at this from the perspective of both the reviewer and the contributor.
As a contributor, after you’ve addressed all the comments from code review and pushed your code make sure to notify the reviewer that the code is ready for another round of review.
Without such a notification, the reviewer has no way of knowing if the code you pushed is still a work-in-progress or the finished product. The reviewer’s only option at this point is to manually reconcile the code you pushed against the comments and make assumptions about the status of the work, which is both inefficient and error-prone.
In GitHub the best way notify the reviewer is to leave a comment on the pull request stating it’s ready for another round of review and “@” the reviewer.
On the flip side of this, as a reviewer, if you see code pushes from the contributor, resist the urge to review and comment on them until the contributor sends a notification that the code is ready for review another round of review.
The code that was pushed may still be in progress and the review may be pointless if the author was planning to further adjust the code. Also, if you’re commenting on code that’s still in progress, it can become very distracting for the contributor to read through and consider those comments while in the midst addressing the original comments received.
The exception to this rule is if the contributor is confused by the reviewer’s comments and further discussion or clarification is needed.
In this case the contributor should notify the reviewer that they are confused by some of the comments. Ideally the discussion would live within GitHub (or where ever the code review is happening) for documentation sake, but sometimes chat, phone or face-to-face conversation is a better medium for such discussion. In these cases it’s always ideal to write up a recap of these discussions and add it source code hosting platform where the review is happening.
In this post I’ve outlined a few simple techniques that can be used to smooth out the code review process. I hope you incorporate these into your project or at your organization.
Hi, I'm Max!
I'm a software developer who mainly works in PHP, but also dabbles in Ruby and Go. Technical topics that interest me are monitoring, security and performance.
During the day I solve challenging technical problems at Something Digital where I mainly work with the Magento platform. I also blog about tech, work on open source and hunt for bugs.
If you'd like to get in touch with me the best way is on Twitter.