4 ms·

On the spectrum of best to worst way to do code reviews - pair programming is the best. Writing comments, no matter how respectful, back and forth is the worst.

On the spectrum of best to worst way to do code reviews - pair programming is the best. Writing comments, no matter how respectful, back and forth is the worst. When I do code reviews I split the difference and "pair code review". My preferred method: I start by realizing the author has had more time to really think it through than I did, give them the benefit of the doubt, and realize I have some faulty assumptions up front (the classic "Building twitter can't be that hard!" type assumptions we see a lot on HN). Then I read through the code on my own but don't post any comments, or only post "uncontroversial comments" (spelling, forgot to scope a query, etc) Then it is time to pair. I start a screen share/video call with the author and let the author take the lead and walk me through it. They tend to find most issues on their own just by talking it out (rubber duck code reviews). Usually the author posts comments on the PR while doing this, not me. When I see a problem I can guide them to the problem with a few prompts and we can discuss it and resolve it right there, without any back and forth in pr comments. Only after doing all this would I consider asking the author to make major changes or re-dos. Sometimes I'm in a hurry and I don't do all this, and it often leads to hurt feels, pr flame wars, etc.

klipt 7y ago
> Sometimes I'm in a hurry and I don't do all this, and it often leads to hurt feels, pr flame wars, etc. Have you considered framing all your suggestions in the form of questions? Eg "have you considered doing this using <X>, because the style guide suggests it/it might be faster/etc" That way if they have already considered it, they can just respond yes, and give the reason they didn't do it. And if they haven't considered it, they can think about it and make a more informed decision. Maybe this is a very passive approach but it seems to work for me. Inspired by a sci-fi writing workshop I took and the tips they gave on how to critique without turning it into a confrontation.
- jacobsenscott 7y ago
  I try. I never say "you". I always "I think" or "I believe", and almost always use questions. I also try and stay fairly terse out of respect for their time and to avoid "talking down" to them. But I think sometimes that comes across wrong. As soon as I sense hurt feelings I jump on a call though.
- strken 7y ago
  Reframing statements as a question is less effective than training yourself to actually ask real questions. If you're 99% convinced that something is wrong, it's no use asking a question like "have you considered not leaving a print statement in the middle of your code?", but if you're only 90% sure then there's already a question you need to ask. This may or may not be what you suggested, sorry if it's redundant.
ThrustVectoring 7y ago
For all but the simplest and most obvious issues, talking through the potential problem is also faster than repeated back-and-forth commentary. I'll still write the comments, but that's more for me to get the thoughts on the issue down on paper than to ensure that my thoughts get communicated accurately and acted upon.
reggieband 7y ago
My best code review experiences, bar none, were at a company that did all code reviews side-by-side at our desks. This was before the days of online code review tools like those built into github - you physically pulled your chair next to the submitters desk and went through the review face-to-face while they showed the diff on their screen. There were times this was cumbersome and intrusive. I tend to make many small changes and I remember a few of my co-workers being frustrated at the frequency I asked for reviews. I remember myself feeling frustrated by the interruption to my "code flow" on occasions. But the benefit of the conversation that happened in those reviews far outweighs any negative I remember. Now that people are socialized to the asynchronous online review process I don't think I can imagine people going back to this synchronous and time consuming process.

klipt 7y ago

> Sometimes I'm in a hurry and I don't do all this, and it often leads to hurt feels, pr flame wars, etc. Have you considered framing all your suggestions in the form of questions? Eg "have you considered doing this using <X>, because the style guide suggests it/it might be faster/etc" That way if they have already considered it, they can just respond yes, and give the reason they didn't do it. And if they haven't considered it, they can think about it and make a more informed decision. Maybe this is a very passive approach but it seems to work for me. Inspired by a sci-fi writing workshop I took and the tips they gave on how to critique without turning it into a confrontation.

jacobsenscott 7y ago
I try. I never say "you". I always "I think" or "I believe", and almost always use questions. I also try and stay fairly terse out of respect for their time and to avoid "talking down" to them. But I think sometimes that comes across wrong. As soon as I sense hurt feelings I jump on a call though.
strken 7y ago
Reframing statements as a question is less effective than training yourself to actually ask real questions. If you're 99% convinced that something is wrong, it's no use asking a question like "have you considered not leaving a print statement in the middle of your code?", but if you're only 90% sure then there's already a question you need to ask. This may or may not be what you suggested, sorry if it's redundant.

ThrustVectoring 7y ago

For all but the simplest and most obvious issues, talking through the potential problem is also faster than repeated back-and-forth commentary. I'll still write the comments, but that's more for me to get the thoughts on the issue down on paper than to ensure that my thoughts get communicated accurately and acted upon.

reggieband 7y ago

My best code review experiences, bar none, were at a company that did all code reviews side-by-side at our desks. This was before the days of online code review tools like those built into github - you physically pulled your chair next to the submitters desk and went through the review face-to-face while they showed the diff on their screen. There were times this was cumbersome and intrusive. I tend to make many small changes and I remember a few of my co-workers being frustrated at the frequency I asked for reviews. I remember myself feeling frustrated by the interruption to my "code flow" on occasions. But the benefit of the conversation that happened in those reviews far outweighs any negative I remember. Now that people are socialized to the asynchronous online review process I don't think I can imagine people going back to this synchronous and time consuming process.