1

In a software development team, when should a developer perform a code review of another developer's task:

  • after completing their own task (another developer's task becomes blocked)

  • as soon as possible (inflicts context switching)

  • a policy is required (e.g. during a day, but also inflicts context switching)

  • I think the question is really an X/Y problem with your workflow, but your assumptions about blocking and task switching are obscurIng the underlying issue. Also, context-switching has a cost, but cost/benefit is always highly situational. – Todd A. Jacobs Jan 14 at 2:42
3

It depends.

It's important to realize that Kanban doesn't have anything to say about code reviews. It's simply a set of tools for visualizing and improving the workflow and flow of work through a workflow. There are a few key concepts - the Kanban board which provides a visualization of the workflow, work on the board, work-in-progress (WIP) limits, work item aging, cycle time, and throughput. These come together to show how work is or isn't flowing from one step to the next all the way through completion.

If you are reaching WIP limits, those will be blocking things from either entering code review or entering the state following code review (such as a ready for test or testing state). The fact that WIP limits are being reached is an indicator to the team what needs to happen. If the WIP limit is in the next column, the team should focus their efforts on getting testing for work finished. If the code review column has its WIP limit reached, then the team should focus on reviewing work.

If you aren't reaching WIP limits, you will need to consider work item aging and your service level expectations. The age of a work item is the amount of time that has elapsed between a work item starting and now. A service level expectation is an estimate of how long it will take for work to be done, often in time and a probability. When deciding what to do, you want to maximize the work that meets your service level expectations, so knowing which items are closest to exceeding those expectations should receive the first attention to move them through the workflow.

Both of these options are based on the reduction of inventory, which is one of the seven wastes from lean. Work-in-progress is one form of inventory. Moving work through the workflow at the expected or desired rate (without compromising quality, of course, since defects and rework are also waste) is one focus of Kanban, and making any kind of bottlenecks or impediments highly visible is how this is achieved.

| improve this answer | |
  • Thank you, but the question isn't about reaching WIP limit. It is about whether a team member should accept context switing caused by a code review request or not. If they are working on a long task then they are putting all code review in a blocked state. – Chris Brettini Jan 13 at 14:02
  • @ChrisBrettini The answer to that is explicitly tied to the WIP limits of columns. If you are doing Kanban, you do not exceed WIP limits. That determines what action you take. You cannot separate those two concepts. – Thomas Owens Jan 13 at 14:04
  • Yes, but if WIP limit is not reached, then should we accept context switching for code review? – Chris Brettini Jan 13 at 14:08
  • @ChrisBrettini I'll update my answer for this case, but to make this determination, the common recommendation is to track work item aging and have a service level expectation. – Thomas Owens Jan 13 at 14:12
  • 1
    @Thomas, while I think your answer is technically correct, it may talk well past many people's understanding of Kanban. It may enhance the answer to make clear that Kanban has nothing to say about code reviews and it is designed to show you where work is flowing and where it isn't, regardless of what those steps are. – Daniel Jan 13 at 16:33
1

TL;DR

Code should be reviewed when someone is ready to review it, and when the team has the capacity to perform the code review without "stopping the line" or impacting other work-in-progress.

Analysis

In any pull-queue system, work is pulled (never pushed) when someone is ready to work on it. So, work should be pulled into your "Code Review" column when:

  1. Someone has a current need for that work item, e.g. they need it to be reviewed now so they themselves (or someone else with a near-term demand) can do something with it shortly thereafter.
  2. Someone has sufficient capacity to take temporary ownership of the work item.
  3. The state into which the work item is being pulled is not currently at the WIP limit for that column.
  4. The rightward shift of all work will not exceed the WIP limit of the board overall. NB: There are use cases where the WIP limit of the Kanban board can be lower than the WIP limit of a given column; in those scenarios, the board limit generally takes precedence over column limits.

When workflows are properly visualized and WIP limits are set appropriately, work may queue up in one state or another, but should rarely (if ever) necessitate the kind of task switching you're describing. Work that requires actively pulling resources from WIP in other states to work on something else generally indicates that WIP somewhere on the board is too high for your current capacity. This should raise immediate concerns, and trigger an inspect-and-adapt cycle to determine why the process isn't flowing smoothly. The team should then optimize working agreements, adjust queues, and right-size WIP limits to reduce or eliminate this type of resource contention in the future.

Recommendations

It seems likely that your board limits are too high. If a task requires two or more people, the WIP limits of other columns needs to be reduced to ensure that there are sufficient resources within the process to allocate to work in that column. For example, given the following WIP limits:

   # Board-Level WIP Limit: 4

  1. To-Do
  2. Development (2)
  3. Code Review (1)
  4. QA (1)
  5. Done

this example Kanban process can have at most 4 work items in progress anywhere on the board. WIP limits say nothing about how many people you need for each work state, but an experienced Kanban practitioner would typically assume a team size of 5-6 people given these column limits in order to provide sufficient slack. Alternatively, a team of four people might set a board-wide limit of three to provide adequate slack at a more-pragmatic capacity of N-1.

Think of it this way: no matter how many states you have, if you have only one person on the team then board_WIP_limit = 1 is your max even if particular columns would otherwise be allowed to queue with values like column_WIP_limit = 2. Your total throughput can't realistically exceed your available resources, so think about the entire process holistically rather than targeting utilization on a per-state basis.

Resource constraints, especially when you have a fixed team size, should generally trigger a process-wide reduction in WIP to optimize throughput. In your specific case, the core recommendation is to stop multiplying tasks by people or states to identify WIP limits. Instead, identify your significant work states, your available team capacity, and then limit work across all states to an aggregate that's less than the team's total capacity.

| improve this answer | |
0

Ask yourself the question - "How do we do this today?". Then map that process to your Kanban board.

Most software/ product teams have people who perform multiple functions - and they are expected to perform these possibly multiple times in a day or depending on how heavy their own workload is, at specific times during the day or week, or even longer - depending on their context.

You could define this using a combination of WIP Limits on your Dev and Review columns which reflect that one or more team members might work on a development task or a review task. If a team member is working on their own dev card - but also have to go review some other developers' work during the day/ week, that would reflect on the Kanban board in two ways -

  1. On the card being reviewed, you might show both the developer and the reviewer "assigned".
  2. The reviewer's own card, could either be shown "Blocked" while they do the review on the other card, or simply waiting in the Dev in-progress column (or perhaps moved to a "Waiting" bin as part of the Dev column. Once they complete the review, they come back to their own card and either unblock it or move it from the 'waiting bin' back to the Dev in-progress column.

Both of these options are shown in the Dev - Ongoing column in the diagram below -

enter image description here

Irrespective of which option you choose, having explicit policies about this is always recommended.

| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.