Code Reviews: Guiding, Not Solving

By: Ryan Kienstra on: October 25, 2017  in: Programming

Computer monitor
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.

Learning Opportunity

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.

But from looking at the documentation, they would see that there are many kinds of internationalization functions. They’d learn about printf(), and maybe sprintf().

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.

Leave a comment