Code Reviews: Guiding, Not Solving
By: Ryan Kienstra on: October 25, 2017 in: Programming
When reviewing code, sometimes I’m tempted to do the work myself. By commenting with the exact code snippet.
Especially in reviews for less experienced developers, it can be faster. And it can prevent a second round of review.
But I think being less clear is better for both of us.
For example, when reviewing this line:
<span>Product Image Number 5</span>
You could suggest the exact solution:
<span><?php printf( esc_html__( 'Product Image Number %d', 'foo-textdomain' ), $image ); ?></span>
But I think it’s usually better to point the reviewee in the right direction:
Please internationalize this, using the $image variable from above.
In the first example, it’s possible that the reviewee would learn nothing from copying and pasting the snippet into the code. A non-developer could do that.
And if they use the PHPCS WordPress Extra ruleset, they’ll see an error:
A gettext call containing placeholders was found, but was not accompanied by a "translators:" comment on the line above to clarify the meaning of the placeholders.
You could have prevented this by adding the right line of code above it. But it would take away the learning of debugging it.
Guiding, Not Solving
I think it’s better to guide developers to find their own solution. The process of learning is just as important as improving the pull request.