Skip to main content
Software Engineering

Return to Answer

added 3 characters in body
Source Link
Joel Etherton
Joel Etherton
  • 11.7k
  • 7
  • 46
  • 55
  1. Get over yourself. You're not perfect, and just because a junior developer managed to see you do something silly in a for loop isn't the end of the world. If you're so great, invite criticism and prove it. Expect code reviews to reveal new ideas on how to do things, new habits, and, most of all, constructive dialog.
  2. Keep an open mind when reviewing someone else's code. Be receptive to their ideas, and make sure that changes you suggest are suggested for a reason. It is assumed that the checked-in code builds, but that doesn't mean that it follows best practices. Make sure that anything you bring up can be backed up with a cited reference. If you say it doesn't follow best practices then cite the standards document and section. If you say it isn't a "performant" method, then have a link to a document that shows why and possibly provides metrics.
  3. Make suggestions useful, and explain why you're suggesting something. You will occasionally find a problem with code that is self explanatory, but most of the time this person has coded something based on habits. Explain why this habit should be altered and the value of altering it (unless you're explaining it for the 5th time, in which you have a personnel problem).
  4. When your code is reviewed, consider everything the reviewer is suggesting objectively. If you're pushing back, ask yourself honestly if you're just being defensive or if you really believe you have a case. If you have a case, continue the dialog. Don't get argumentative, bring ammunition such as facts and metrics.
  5. Whether you're a reviewer or a reviewee, use code reviews as an opportunity to educate. Whether it's educating yourself or the person you're reviewing, if there is a discrepancy then there is a chance to learn somewhere. Make good use of it.
  6. Ask questions and be ready to have your questions answered truthfully. I recently made a statement that was "pseudo-true". It wasn't wrong, but it definitely wasn't right. A junior developer challenged me on it, and I disagreed. My response was "I haven't seen that behavior, but if you can find me a document on it I would love to read it.". I spent about an hour that afternoon reading the document he sent me, and now I have a much better response (re: educated) when confronted with a similar situation.
  1. Get over yourself. You're not perfect, and just because a junior developer managed to see you do something silly in a for loop isn't the end of the world. If you're so great, invite criticism and prove it. Expect code reviews to reveal new ideas on how to do things, new habits, and, most of all, constructive dialog.
  2. Keep an open mind when reviewing someone else's code. Be receptive to their ideas, and make sure that changes you suggest are suggested for a reason. It is assumed that the checked-in code builds, but that doesn't mean that it follows best practices. Make sure that anything you bring up can be backed up with a cited reference. If you say it doesn't follow best practices then cite the standards document and section. If you say it isn't a "performant" method, then have a link to a document that shows why and possibly provides metrics.
  3. Make suggestions useful, and explain why you're suggesting something. You will occasionally find a problem with code that is self explanatory, but most of the time this person has coded something based on habits. Explain why this habit should be altered and the value of altering it (unless you're explaining it for the 5th time, in which you have a personnel problem).
  4. When your code is reviewed, consider everything the reviewer is suggesting objectively. If you're pushing back, ask yourself honestly if you're just being defensive or if you really believe you have a case. If you have a case, continue the dialog. Don't get argumentative, bring ammunition such as facts and metrics.
  5. Whether you're a reviewer or a reviewee, use code reviews as opportunity to educate. Whether it's educating yourself or the person you're reviewing, if there is a discrepancy then there is a chance to learn somewhere. Make good use of it.
  6. Ask questions and be ready to have your questions answered truthfully. I recently made a statement that was "pseudo-true". It wasn't wrong, but it definitely wasn't right. A junior developer challenged me on it, and I disagreed. My response was "I haven't seen that behavior, but if you can find me a document on it I would love to read it.". I spent about an hour that afternoon reading the document he sent me, and now I have a much better response (re: educated) when confronted with a similar situation.
  1. Get over yourself. You're not perfect, and just because a junior developer managed to see you do something silly in a for loop isn't the end of the world. If you're so great, invite criticism and prove it. Expect code reviews to reveal new ideas on how to do things, new habits, and, most of all, constructive dialog.
  2. Keep an open mind when reviewing someone else's code. Be receptive to their ideas, and make sure that changes you suggest are suggested for a reason. It is assumed that the checked-in code builds, but that doesn't mean that it follows best practices. Make sure that anything you bring up can be backed up with a cited reference. If you say it doesn't follow best practices then cite the standards document and section. If you say it isn't a "performant" method, then have a link to a document that shows why and possibly provides metrics.
  3. Make suggestions useful, and explain why you're suggesting something. You will occasionally find a problem with code that is self explanatory, but most of the time this person has coded something based on habits. Explain why this habit should be altered and the value of altering it (unless you're explaining it for the 5th time, in which you have a personnel problem).
  4. When your code is reviewed, consider everything the reviewer is suggesting objectively. If you're pushing back, ask yourself honestly if you're just being defensive or if you really believe you have a case. If you have a case, continue the dialog. Don't get argumentative, bring ammunition such as facts and metrics.
  5. Whether you're a reviewer or a reviewee, use code reviews as an opportunity to educate. Whether it's educating yourself or the person you're reviewing, if there is a discrepancy then there is a chance to learn somewhere. Make good use of it.
  6. Ask questions and be ready to have your questions answered truthfully. I recently made a statement that was "pseudo-true". It wasn't wrong, but it definitely wasn't right. A junior developer challenged me on it, and I disagreed. My response was "I haven't seen that behavior, but if you can find me a document on it I would love to read it.". I spent about an hour that afternoon reading the document he sent me, and now I have a much better response (re: educated) when confronted with a similar situation.
Post Migrated Here from workplace.stackexchange.com (revisions)
Source Link
Joel Etherton
Joel Etherton
  • 11.7k
  • 7
  • 46
  • 55

This seems to be a pretty common prevailing attitude among some developers. Everyone seems to feel that a code review is some challenge to their work, and that makes no sense to me. A code review is a quality assurance mechanism that has the added bonus of education to go along with it. We implement code reviews extensively where I work, and I've fostered within my own team the attitude that the code reviews are a collaboration mechanism more than a quality process.

The only way to begin coding as a team is to see each other's work and to question it. That's how best practices are formed. Dialog is the key. I've sent code back to developers for silly reasons such as formatting, agreed upon best practices, and spelling. I code review with a very fine edge, and I expect my own code to withstand the same scrutiny. I've used tactics such as checking in code that would not pass my own code review for the sole purpose of getting a junior developer to challenge me. Tell me it doesn't meet the requirements. Stand up and have an opinion.

Some guidelines I think everyone should really have when engaged in a code reviewing environment:

  1. Get over yourself. You're not perfect, and just because a junior developer managed to see you do something silly in a for loop isn't the end of the world. If you're so great, invite criticism and prove it. Expect code reviews to reveal new ideas on how to do things, new habits, and, most of all, constructive dialog.
  2. Keep an open mind when reviewing someone else's code. Be receptive to their ideas, and make sure that changes you suggest are suggested for a reason. It is assumed that the checked-in code builds, but that doesn't mean that it follows best practices. Make sure that anything you bring up can be backed up with a cited reference. If you say it doesn't follow best practices then cite the standards document and section. If you say it isn't a "performant" method, then have a link to a document that shows why and possibly provides metrics.
  3. Make suggestions useful, and explain why you're suggesting something. You will occasionally find a problem with code that is self explanatory, but most of the time this person has coded something based on habits. Explain why this habit should be altered and the value of altering it (unless you're explaining it for the 5th time, in which you have a personnel problem).
  4. When your code is reviewed, consider everything the reviewer is suggesting objectively. If you're pushing back, ask yourself honestly if you're just being defensive or if you really believe you have a case. If you have a case, continue the dialog. Don't get argumentative, bring ammunition such as facts and metrics.
  5. Whether you're a reviewer or a reviewee, use code reviews as opportunity to educate. Whether it's educating yourself or the person you're reviewing, if there is a discrepancy then there is a chance to learn somewhere. Make good use of it.
  6. Ask questions and be ready to have your questions answered truthfully. I recently made a statement that was "pseudo-true". It wasn't wrong, but it definitely wasn't right. A junior developer challenged me on it, and I disagreed. My response was "I haven't seen that behavior, but if you can find me a document on it I would love to read it.". I spent about an hour that afternoon reading the document he sent me, and now I have a much better response (re: educated) when confronted with a similar situation.

Given the education slant that I put code reviews in, I often make my responses about questions. If I find something blatantly wonky, I will instruct the developer to correct it. Otherwise I will ask the questions. "Why did you use method A to achieve goal B?" "What gain does declaring a variable have in the instance of its usage in method Z?" "I see you have copied/pasted some code, did you consider refactoring? You didn't refactor, what was the reasoning behind that choice?" The code doesn't progress until the reviewer approves it, and the reviewer won't approve it until the questions are answered. When framed in an inquisitive way that indicates you don't understand the developers reasoning it becomes less confrontational and takes on more of an instructional vibe.