Advice to Code Reviewers

I mean, take your time in writing your comments. I often edit a comment three times to make sure it is kind and respectful. Since we all take pride in our code, reviews can make any of us a bit sensitive.

Make sure your suggestion is clearly explained, as hastily written comments may be misunderstood, causing unnecessary rewrites, pull requests, further reviews.

Review comments don’t have to only be about improvements. Pat your coworker on the back, and call out the good code and ideas associated with the changes.

My new approach that I’m starting is leaving little beer emojis to indicate that I owe that teammate a beer for doing writing a particular gem, or solving a bug that has plagued me.

While the top-level engineering promotions at most companies require a significant amount of mentoring, since you are really intelligent, you should get into the practice of mentoring others anyway. As such, start including more “why” explanations, including code examples, deep links into style guides, and references to existing code that may illustrate your point.

I think we’ve all been on the steeper parts of technological learning curve and received a “do it my way” review with zero justification. By explaining the “why”, not only does the code gets better, the team gets better too!

Careful. Do not go to the dark side with this suggestion and start ’splaining (I think you know what I’m referring to). Perhaps we should change the term from mentor to sensei, as one definition of that term I’ve heard is: one who is further ahead on the path; which characterizes a teacher as one who can look back along a path and say, “Watch your step there, that’s a particularly slippery spot. If you do fall, it’s okay, because I fell too.” It’s a compassionate approach to teaching, which we should all adopt.

Now that I’m experienced (I’m not quite old just yet), I can look back at my life and see how many times I was absolutely certain about many things, that I now see to be completely incorrect…laughably wrong. Sure, I should expect my experience should be more valuable now, but I still might be wrong.

You can be wrong too. Obviously, not all the time, but you can never be too sure. Don’t take this advice as an urge to limit your comments, but in the conversation that occurs, recognize your teammates perspective is valid too.

We can agree that writing code is quite subjective, almost artistic, right? This means we have many ways of express a solution to a problem in code, so be open to other approaches.

Time for a story. Every time I run the linter on my Python code, it complains bitterly about my use of map, filter and reduce, and claims I should use a comprehension. Perhaps this may date me, but these functions are my bread and butter, as they seem so clear to me. Comprehensions, on the other hand, seem verbose and clutter intention. However, I usually rewrite them as my other colleagues agree with the linter and feel it is easier to read.

If you had posted some code, which of the following three comments would you rather see?

Inline this function, please.

Or:

Since the function called is a single line, and only called here, let’s inline the function.

Or:

Do you think that since this function is a single line, and only called here, would it be more understandable to inline it?

While they all state the same request, I’d argue the last one is the nicest. First, since we’ve already established that we might not have all the context and be wrong, asking a question not only softens the request, but initiates a dialog, not a response.

The other feature it brings is an ego-supporting back-door for both of you. Since this isn’t a direct command, the author may be more willing to accept the comment, but if the author responds with clarifying information why it should stay, you too, are more apt to accept it since it was just a question.

This tip is golden, and I often rephrase all my suggestions as questions.

I’m sure that Mrs. McDougal told you in 8th Grade English class to never use the passive voice and to rewrite your sentence with an active participant, I feel the passive voice deflects criticism from the author and to the code.

Also substitute we for you, as in:

Add a comment to the complex algorithm you wrote.

To something like:

With such a complex algorithm, we should add a clarifying comment.

Granted, we could apply all these tips, to really begin a discussion:

With such an innovative approach to solving this problem, should we add a clarifying comment explaining it, or should we break out some of the expressions into functions that we could name to help the reader follow the logical flow?

While this may be obvious, our goal should always be to foster and grow a team, so don’t leave snarky or condescending comments. Even if you are the team lead with ultimate responsibility for code quality, outright commands may seem heavy-handed, and build underlying resentment.

Also, be careful with the jokes, and no, adding an emoji doesn’t make a dig funny.

I wrote this to start a discussion. What advice would you suggest to help further the goals of code quality as well as team building?

Thanks to Ken for reviewing this and your wisdom… oh, and for the great definition of sensei.