4 ms·

I’ve found reviewing the code together face to face (or video call) with author is very effective. It allows you to discuss why they made certain decisions, why

I’ve found reviewing the code together face to face (or video call) with author is very effective. It allows you to discuss why they made certain decisions, why you recommend different choices, and the relative benefits of each option. It also the best forum to remind them that my code isn’t perfect either, that this is a process to help make us both better developers.

Sourya 8y ago
I've had my code reviewed face-to-face when I was just beginning my career, and I think that it has helped me learn much more than I would have otherwise. At least for someone who has never written production-ready code (first internship / first job etc), it is hard to understand why a particular piece of code is "bad design" even though it seems correct logically.
deleted 8y ago
[deleted]
dolftax 8y ago
The only problem I see is the synchronous nature. The reviewer and author should be active at the same time. But yes, it helps.
rootusrootus 8y ago
As an alternative, I encourage developers on my team to liberally sprinkle comments on their own PRs which explain their logical path. Not only does it help me understand where they were going with something, it also serves as a bit of 'rubber duck' debugging -- more than once as I've explained a bit of my own code I've seen obvious ways it could be more elegant, or I've spotted potential bug situations.
eequah9L 8y ago
Online code review puts pressure on the reviewer to find the problems quickly so as not to take the other engineer's time. Usually I want to take my sweet time, look at the surrounding code, figure out things for myself. Sometimes I just want to stare at the screen for five minutes to think about it. That's really uncomfortable when there is another person on the other end of the line. If something is so unclear that I cannot figure it out on my own, the code / commit message / cover letter likely need to be improved. Nothing like git-blaming your way to a two-year old commit and get a commit message that actually explains the whys and trade-offs. I agree that some things do need discussion, but code review tools typically have some sort of chat interface for that purpose. I am doing open source work and most of the time just do code review over the e-mail, which obviously works well for this purpose. [Edit: and of course the calls work as well here! In any case, the result of the discussion should make its way to the code / commit messages / cover letter.]
- EB66 8y ago
  > Online code review puts pressure on the reviewer to find the problems quickly so as not to take the other engineer's time. Usually I want to take my sweet time, look at the surrounding code, figure out things for myself. Sometimes I just want to stare at the screen for five minutes to think about it. That's really uncomfortable when there is another person on the other end of the line. Yes, absolutely this. I think it's essential that the reviewer takes their time to inspect the code without distractions and without being rushed. If you want to have a face-to-face code review, in my experience it works better to have that meeting after finishing the code review, so you can go over your review and discuss each point in greater detail.
  - valuearb 8y ago
    These are both good points that I neglected. In fact that's actually what I do, I never do a face to face unprepared, I always have a list of things I've found that I want to review. Another thing that I maybe didn't make clear enough was it's important to be emphatic, and treat them as equal team-members, even in a Sr to Jr review. Give them an opportunity to weigh in on why they made certain choices and discuss all the pros/cons. Sometimes I even change my mind and agree with their choices when it's clearer what their objectives were.

Sourya 8y ago

I've had my code reviewed face-to-face when I was just beginning my career, and I think that it has helped me learn much more than I would have otherwise. At least for someone who has never written production-ready code (first internship / first job etc), it is hard to understand why a particular piece of code is "bad design" even though it seems correct logically.

deleted 8y ago

[deleted]

dolftax 8y ago

The only problem I see is the synchronous nature. The reviewer and author should be active at the same time. But yes, it helps.

rootusrootus 8y ago

As an alternative, I encourage developers on my team to liberally sprinkle comments on their own PRs which explain their logical path. Not only does it help me understand where they were going with something, it also serves as a bit of 'rubber duck' debugging -- more than once as I've explained a bit of my own code I've seen obvious ways it could be more elegant, or I've spotted potential bug situations.

eequah9L 8y ago

Online code review puts pressure on the reviewer to find the problems quickly so as not to take the other engineer's time. Usually I want to take my sweet time, look at the surrounding code, figure out things for myself. Sometimes I just want to stare at the screen for five minutes to think about it. That's really uncomfortable when there is another person on the other end of the line. If something is so unclear that I cannot figure it out on my own, the code / commit message / cover letter likely need to be improved. Nothing like git-blaming your way to a two-year old commit and get a commit message that actually explains the whys and trade-offs. I agree that some things do need discussion, but code review tools typically have some sort of chat interface for that purpose. I am doing open source work and most of the time just do code review over the e-mail, which obviously works well for this purpose. [Edit: and of course the calls work as well here! In any case, the result of the discussion should make its way to the code / commit messages / cover letter.]

EB66 8y ago
> Online code review puts pressure on the reviewer to find the problems quickly so as not to take the other engineer's time. Usually I want to take my sweet time, look at the surrounding code, figure out things for myself. Sometimes I just want to stare at the screen for five minutes to think about it. That's really uncomfortable when there is another person on the other end of the line. Yes, absolutely this. I think it's essential that the reviewer takes their time to inspect the code without distractions and without being rushed. If you want to have a face-to-face code review, in my experience it works better to have that meeting after finishing the code review, so you can go over your review and discuss each point in greater detail.
- valuearb 8y ago
  These are both good points that I neglected. In fact that's actually what I do, I never do a face to face unprepared, I always have a list of things I've found that I want to review. Another thing that I maybe didn't make clear enough was it's important to be emphatic, and treat them as equal team-members, even in a Sr to Jr review. Give them an opportunity to weigh in on why they made certain choices and discuss all the pros/cons. Sometimes I even change my mind and agree with their choices when it's clearer what their objectives were.