How to treat code review comments

When you send out changes/pull requests and get revision comments, what should you do? Especially, if “quick work-arounds” exist for some issues, what will you do?

It’s tempting to apply the “quick work-around”, and get the changes submitted ASAP, since you will have get something DONE and cross it off your list. You are not against making some larger changes, but would want them to be separated changes. “Iteration”, you say.

It’s not wrong to do things that way, but DO remember to finish the “larger change”, or at least open an issue, triage to the correct priority, and do it when time comes. Otherwise, you’ll have left some tech debt/potential issues in your code base that’s hidden and forgotten, until one day it comes back to bite you. Not only does it hurt your project/codebase, it hurts your teammates’ trust in you.

Reading kindness is underrated, I realize what happened when I submitted changes without fully addressing comments. It’s nice that I got something done, and would enjoy the feeling of accomplishment for the moment. But it hurts your team culture and efficiency in longer term.

  1. Teammate T will feel ignored and/or unrespected.
  2. Next time making a comment, T will need to emphasize things in a more direct manner with a stronger opinion, which may make me feel more nervous in return.
  3. I become more cautious, but meanwhile less comfortable working with T, vice versa

In this process, trust is weakened. It will reach a balance at one point, where both T and me will have weaker trust in each other and less efficient communication. This is not ideal.

To break the loop, a few things to do:

  1. DO read and address comments. Don’t skip/ignore. Don’t just focus on getting the thing done. Get it done well, up to expectation, or make sure you do complete what you said in TODO.
  2. Don’t have hard feelings on comments. Sometimes people are trying to make sure you get their point, if you react proactively they may be softer in future. Other times, you have met someone not nice enough or having a bad day. Just get the right thing done, and their emotion/communication style is not your fault. Get used to it.

Leave a Reply

Your email address will not be published. Required fields are marked *

To create code blocks or other preformatted text, indent by four spaces:

    This will be displayed in a monospaced font. The first four 
    spaces will be stripped off, but all other whitespace
    will be preserved.
    
    Markdown is turned off in code blocks:
     [This is not a link](http://example.com)

To create not a block, but an inline code span, use backticks:

Here is some inline `code`.

For more help see http://daringfireball.net/projects/markdown/syntax