7 ms·

> If code reviews are absolute blockers on your team then your team is dysfunctional So what am I supposed to work on when I'm waiting for my code to be review

> If code reviews are absolute blockers on your team then your team is dysfunctional So what am I supposed to work on when I'm waiting for my code to be reviewed? Am I supposed to context-switch over to another task, losing all of my current built-up state about the current problem and making it take twice as long just because you think your task is more important? Async reviews slow the whole team down, because juggling more tasks in your head means that you can't retain the high-context information about all of them as easily and you lose a lot of time to context-switching. In the synchronous review model, only one team member has to do a context switch (and a much lighter one than a full context switch to work on another task). In addition, synchronous reviews over a voice/video chat just take significantly less time in general, because being able to ask "Why did you do it this way?" and have an immediate discussion about the trade-offs is strictly superior to having to have that same discussion back and forth over days of async responses.

jenscow 3y ago
What about the reviewer's "context-switch" to look at your precious code?
- nightpool 3y ago
  As I said, context-switching and interrupting my work to review code is going to be less of an impact then setting down a task entirely to work on implementing a completely different task. Reviewing already written code just doesn't require as much context as writing the code from scratch, and the other engineer (in theory) has already done the job of putting all the context into the pull request description so you can review it easily.
  - jenscow 3y ago
    Then the same applies to you for receiving a review. But anyway, needing everyone into an office because you struggle isn't the fault of WFH
    - nightpool 3y ago
      No, it doesn't. What I'm saying is that context-switching what you're actually coding on to pick up a new task is a more costly endeavor than just context-switching in or out of a review. In the synchronous model, there are only two tasks involved—one per employee. In the asynchronous model, you need to pick up a second task to work on while you're waiting for me to review your code. So you'll need to context-switch fully onto the other task to work on it, and you're probably going to get less work done than if you can devote all of your attention to one task at a time. This isn't about "me struggling", it's about ways the entire team's productivity can be affected by choosing an asynchronous vs synchronous code review model. https://www.atlassian.com/blog/productivity/context-switching#:~:text=2.%20It%E2%80%99s%20easier,to%20crash.%20Exhausting https://www.atlassian.com/blog/productivity/context-switchin...! has some more info on how certain types of context switches can be more damaging than others. I've been working remote for 3 years now with a synchronous code review system, and I wouldn't give it up for the world.
      - jenscow 3y ago
        But to get any real value from a review, the reviewer will need to context switch. Reviewing code together is reasonable, but having to review it as soon as you say "done it" is an interruption.
erik_seaberg 3y ago
If it took you a day to write a branch, do you really want to sit quietly watching me read and think for an hour? Anyway, everything I can’t understand without asking you needs to get written down for the on-call eng next year.
- nightpool 3y ago
  > If it took you a day to write a branch, do you really want to sit quietly watching me read and think for an hour? Yes, absolutely, if that hour is going to be productive and collaborative in coming up with ways to make the code better (which every in-person code review I've been in has been) > everything I can’t understand without asking you needs to get written down for the on-call eng next year No, everything that you can't understand without asking me should be fixed by both of us so that it can be understood by everybody. Approving hard to understand code just because it's been "documented" hurts the entire codebase.
fzeroracer 3y ago
> Am I supposed to context-switch over to another task, losing all of my current built-up state about the current problem and making it take twice as long just because you think your task is more important? Correct, yes. Your current problem is at a state of being done, beyond any code review feedback you may receive. Expecting people to immediately cancel everything they're doing and review your code is not only absurd but also leads to poor quality code reviews. Because rather than actually think about the code and spend time on it, you want them to approve it as fast as possible so you are unblocked as soon as possible. > Async reviews slow the whole team down, because juggling more tasks in your head means that you can't retain the high-context information about all of them as easily and you lose a lot of time to context-switching. The reviewer has to context switch and juggle multiple tasks. They have to understand what your code does, why it does it and why it was necessary in the first place. They have to open the ticket, read what it's about, read any conversations and then figure out any potential concerns. > In the synchronous review model, only one team member has to do a context switch (and a much lighter one than a full context switch to work on another task). If you have one person review your code then I question that your code review process even matters. For extensive reworks, redesigns etc you are going to pull in more individuals and there is going to be more time taken. > In addition, synchronous reviews over a voice/video chat just take significantly less time in general, because being able to ask "Why did you do it this way?" and have an immediate discussion about the trade-offs is strictly superior to having to have that same discussion back and forth over days of async responses. They take far more time in my experience. I've had review turnaround be a few hours to a day in an async context for major reviews. They review the code, we hop on a call when convenient and go over the issues in 15m and we're done. If it's nothing major or understandable over text, then comments are good enough.

jenscow 3y ago

What about the reviewer's "context-switch" to look at your precious code?

nightpool 3y ago
As I said, context-switching and interrupting my work to review code is going to be less of an impact then setting down a task entirely to work on implementing a completely different task. Reviewing already written code just doesn't require as much context as writing the code from scratch, and the other engineer (in theory) has already done the job of putting all the context into the pull request description so you can review it easily.
- jenscow 3y ago
  Then the same applies to you for receiving a review. But anyway, needing everyone into an office because you struggle isn't the fault of WFH
  - nightpool 3y ago
    No, it doesn't. What I'm saying is that context-switching what you're actually coding on to pick up a new task is a more costly endeavor than just context-switching in or out of a review. In the synchronous model, there are only two tasks involved—one per employee. In the asynchronous model, you need to pick up a second task to work on while you're waiting for me to review your code. So you'll need to context-switch fully onto the other task to work on it, and you're probably going to get less work done than if you can devote all of your attention to one task at a time. This isn't about "me struggling", it's about ways the entire team's productivity can be affected by choosing an asynchronous vs synchronous code review model. https://www.atlassian.com/blog/productivity/context-switching#:~:text=2.%20It%E2%80%99s%20easier,to%20crash.%20Exhausting https://www.atlassian.com/blog/productivity/context-switchin...! has some more info on how certain types of context switches can be more damaging than others. I've been working remote for 3 years now with a synchronous code review system, and I wouldn't give it up for the world.
    - jenscow 3y ago
      But to get any real value from a review, the reviewer will need to context switch. Reviewing code together is reasonable, but having to review it as soon as you say "done it" is an interruption.

erik_seaberg 3y ago

If it took you a day to write a branch, do you really want to sit quietly watching me read and think for an hour? Anyway, everything I can’t understand without asking you needs to get written down for the on-call eng next year.

nightpool 3y ago
> If it took you a day to write a branch, do you really want to sit quietly watching me read and think for an hour? Yes, absolutely, if that hour is going to be productive and collaborative in coming up with ways to make the code better (which every in-person code review I've been in has been) > everything I can’t understand without asking you needs to get written down for the on-call eng next year No, everything that you can't understand without asking me should be fixed by both of us so that it can be understood by everybody. Approving hard to understand code just because it's been "documented" hurts the entire codebase.

fzeroracer 3y ago

> Am I supposed to context-switch over to another task, losing all of my current built-up state about the current problem and making it take twice as long just because you think your task is more important? Correct, yes. Your current problem is at a state of being done, beyond any code review feedback you may receive. Expecting people to immediately cancel everything they're doing and review your code is not only absurd but also leads to poor quality code reviews. Because rather than actually think about the code and spend time on it, you want them to approve it as fast as possible so you are unblocked as soon as possible. > Async reviews slow the whole team down, because juggling more tasks in your head means that you can't retain the high-context information about all of them as easily and you lose a lot of time to context-switching. The reviewer has to context switch and juggle multiple tasks. They have to understand what your code does, why it does it and why it was necessary in the first place. They have to open the ticket, read what it's about, read any conversations and then figure out any potential concerns. > In the synchronous review model, only one team member has to do a context switch (and a much lighter one than a full context switch to work on another task). If you have one person review your code then I question that your code review process even matters. For extensive reworks, redesigns etc you are going to pull in more individuals and there is going to be more time taken. > In addition, synchronous reviews over a voice/video chat just take significantly less time in general, because being able to ask "Why did you do it this way?" and have an immediate discussion about the trade-offs is strictly superior to having to have that same discussion back and forth over days of async responses. They take far more time in my experience. I've had review turnaround be a few hours to a day in an async context for major reviews. They review the code, we hop on a call when convenient and go over the issues in 15m and we're done. If it's nothing major or understandable over text, then comments are good enough.