4 ms·

In theory a code review should be a conversation in which you reach consensus. The goal should be that the reviewer understands the code and what it does and i

In theory a code review should be a conversation in which you reach consensus. The goal should be that the reviewer understands the code and what it does and is satisfied with the other things. In reality what often happens is a busy programmer looks at it, spots that there is a { in the wrong place, flags that and considers his/her job done and moves on. If someone flags something in a way that you disagree/or don't understand the best approach is "Can you explain why you think your approach is better than mine and what the trade-offs are?" in which case you'll either get an answer that is satisfactory and accept their changes or not in which case you then have to escalate it again. If you think of it as asynchronous pair programming rather than a chance for someone to nitpick everything you do then it's more obvious how to respond.

MichaelMoser123 7y ago
At what stage of the argument is one going to be flagged as uncooperative?
- noir_lord 7y ago
  At whatever point one party pisses off another in some way. You can delay that point by been open and reasonable up to a point, if you can't then reach consensus you need to escalate it to a lead/PM. It can get both messy and personal if you aren't careful because two programmers might often solve a problem equally well but differently and therefore there simply isn't a clear correct answer. In those cases I'd tend to default to the solution of the person who wrote the code (simply because they'll have put more thought into each part than the person doing the review usually). Team dynamics will largely play a part as well, you may have an incredibly good programmer who can't take criticism at all well (seen that) or whatever and then it becomes a management problem rather than a technical problem. A lot of it is simply tone and empathy though, "I'm curious why you did A instead of B here?" against "Why the hell didn't you do B?" or "B is clearly superior!" (with no explanation of why or the trade-offs). Asking clarifying questions is often a good way to drill down to the meat of the problem (and in a good code review environment imo one of the strongest benefits, it increases comprehension of the code and intent for all parties). One of my lecturers used to say "there are no technical solutions to a people problem".
  - MichaelMoser123 7y ago
    Thanks! So one possible answer is: try to reach the other party directly as soon as possibles (directly so that the discussion is not logged as responses to the code review log), this might help to reach consensus without stepping on anyones toes, so to speak. (and before the other party feels like it wants to escalate the issue) I think it is better to keep the discussions like this off the record, because the other party might not feel confident to formulate his/her position in a formal manner or might feel insecure about having this kind of paper trail .

MichaelMoser123 7y ago

At what stage of the argument is one going to be flagged as uncooperative?

noir_lord 7y ago
At whatever point one party pisses off another in some way. You can delay that point by been open and reasonable up to a point, if you can't then reach consensus you need to escalate it to a lead/PM. It can get both messy and personal if you aren't careful because two programmers might often solve a problem equally well but differently and therefore there simply isn't a clear correct answer. In those cases I'd tend to default to the solution of the person who wrote the code (simply because they'll have put more thought into each part than the person doing the review usually). Team dynamics will largely play a part as well, you may have an incredibly good programmer who can't take criticism at all well (seen that) or whatever and then it becomes a management problem rather than a technical problem. A lot of it is simply tone and empathy though, "I'm curious why you did A instead of B here?" against "Why the hell didn't you do B?" or "B is clearly superior!" (with no explanation of why or the trade-offs). Asking clarifying questions is often a good way to drill down to the meat of the problem (and in a good code review environment imo one of the strongest benefits, it increases comprehension of the code and intent for all parties). One of my lecturers used to say "there are no technical solutions to a people problem".
- MichaelMoser123 7y ago
  Thanks! So one possible answer is: try to reach the other party directly as soon as possibles (directly so that the discussion is not logged as responses to the code review log), this might help to reach consensus without stepping on anyones toes, so to speak. (and before the other party feels like it wants to escalate the issue) I think it is better to keep the discussions like this off the record, because the other party might not feel confident to formulate his/her position in a formal manner or might feel insecure about having this kind of paper trail .