0

In our team we have a recurrent problem that we are not able to solve. As long as the team grows, we tend to accumulated PRs at the end of the sprint / release and these become a bottleneck to advance in our daily tasks, and at the same time, at the end of the sprint we have to let our work to review PRs.

Our goal is to get to the end of the sprint / release with the most of PRs reviewed, but the team does not review PRs during the sprint (due their daily tasks, their work and issues)

I post this problem because I would like to know the strategies other teams follows in order to aboard the Pull request management

Our team is composed by 7 developers and the PRs needs at least 2 approvals to be merged

Thanks

2
  • 5
    If you don't have the time to do something, but it needs to be done, then you have to make the time to do the thing. I fail to see how this is a problem, let alone one focusing on software engineering.
    – Flater
    Mar 17, 2022 at 13:14
  • 7
    "the team does not review PRs ... due their daily tasks" - if you're using a PR-based workflow, PR reviews are some of your daily tasks.
    – jonrsharpe
    Mar 17, 2022 at 13:29

3 Answers 3

8

Reviewing pull requests is part of your team's work. If you as a team decided that PRs are needed (and that they need to be reviewed by 2 developers), then they are just as important as writing the code itself.

If you have PRs piling up at the end of the sprint, there is actually plenty of time available to review them during the sprint - the team just has to do that instead of writing more code that won't get reviewed by the end of the sprint.

If you are working in an agile / scrum environment, lack of PR review is something that should be brought up as a blocker during the daily standup. Bring it up every day that the reviews are not completed.

1
  • 2
    Our team does the same. An open code review/pull request is a blocker -- not just for the person who submitted the pull request. It blocks the entire story, too. QA can also help speed things up, because an open pull request blocks QA as well. If the team begins QA testing before the pull request has been reviewed, stop that practice immediately. If QA isn't given sufficient time to test new work, QA needs to communicate this. And by golly I bet people start reviewing pull requests in a more timely manner. Mar 17, 2022 at 14:44
2

I would like to echo the recommendations in the answer provided by mmathis. A pull request is a blocker, and a task is not complete until the pull request is approved. I will expand a little on some team process adjustments that might help as well.

Developers get busy. Code reviews involve switching contexts, and it takes time for people's minds to switch from their task, to reviewing code, and then back to their task. To reduce the impact of context switching, you can make some recommendations about when to review pull requests.

Take advantage of natural breaks in your day:

  • When your workday first starts
  • After any scheduled meeting
  • Right after lunch or after a break
  • At the end of the workday

Still, this only provides a handful of times when pull requests get reviewed. It could take 2-4 hours before a pull request gets the appropriate number of approvals. When a task only took 2-4 hours to get into code review, this introduces a lot of lag time just waiting for people to review code. Part of the solution involves setting expectations for how long pull requests should take.

My team ran into this problem a few months back. One third to one half of the time it took to complete a task was spent in code review. We had to update our team rules about how long pull requests should be open before getting feedback:

  • All pull requests should have at least 1 approval or 1 rejection within 20 minutes of submitting the pull request.
  • The pull request should be fully approved or rejected within 40 minutes.
  • If you submit a pull request, and have not gotten feedback within 20 minutes, post a message to the entire team reminding them you are blocked.

These guidelines worked well for us, because code reviews are typically quite small. You may choose whatever criteria suites your team. This likely involves meeting with them to address the problem, and have the team come up with a solution.

2
  • 1
    20 minutes sounds a bit low. Depending on the size and content of the PR, it could take 20 minutes to read and comment. Also, when developer A finishes his task and creates a PR, his colleague B might be in the middle of a complex problem and would not appreciate getting interrupted. Don't your developers have anything they can do while waiting for their code to be reviewed?
    – Llewellyn
    Mar 22, 2022 at 22:02
  • 1
    I'm not a fan of these hard limits on time, simply because context matters. For properly separated feature branches, it might not matter to review it immediately if the current sprint has no further work that depends on the existence of feature 3. It would be counterproductive to force your developers to drop what they're doing and immediately review a branch who no one is particularly waiting for or blocked by. If the lack of review/merge is blocking, then a developer should be able to alert their team (and have them respond) without needing these strict time limit rules to be in place.
    – Flater
    Mar 23, 2022 at 10:26
1

My first suggestion would be to identify why the team is using gating pull requests. Depending on the intent, there may be alternative solutions that can achieve the same outcomes without hard gates that prevent work from being integrated in a timely manner.

I'd also look at having a goal to have most of the pull requests reviewed. You should have a clear and consistent definition of what it means for work to be done. Scrum calls this the Definition of Done. In Kanban, the Definition of Workflow has explicit policies around starting and finishing work items. Other methodologies have similar components. Either the pull request is required to complete the work or its not. You use this definition during planning. If you are working in timeboxed iterations, you should plan on a body of work that is achievable within the timebox. If you require the pull request to be reviewed before the work is considered done, then you need to consider that from the point of refining and estimating the work through planning through your daily execution.

You can also take steps to reduce the human burden of reviews. Using code linters, especially those that can be automated, can flag and even fix stylistic issues before the code review starts. Static analysis tools can flag potential problems before the code review and generate reports for the code reviewer to look at to see if they are false positives. Creating automated tests before the code review and including them in the review can express the intention behind the changes and help the code reviewer focus on making sure the intention is correct. By letting the reviewers focus on what humans need to focus on - correctness of the requirements, readability, maintainability - review duration can be shortened.

1
  • I had a linter introduce a bug in my code. I wasn’t happy.
    – gnasher729
    Mar 23, 2022 at 7:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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