DEV Community

Discussion on: What is the "best" merge request flow?

Collapse
 
cassandraspruit profile image
Cassandra Spruit

If everyone's too busy or don't check their messages enough to approve a pull request, that's probably an indicator of other problems on a team.

Here's some tips:

Team members are too busy
This is actually a bit of hard one since "too busy" is a bit nebulous and - let's face it - all too common.

  • Is this because of a bottleneck? Do you require a manager or a specific person to sign off? If that's the case, is it really necessary? Can it be shifted off to another person or pushed later in the development cycle?

And sometimes the answer is no - so maybe ask that person to set aside time a scheduled time during the day/week so they don't pile up. Beginning or end of the day before people get into their day works well.

  • Are people busy because they are...well, busy? The scheduling trick works for groups of people too. During some heavy crunch times where everyone's bogged down, we've had to move a "team review time" where everyone who possibly could took a half hour to do code reviews and such. It also helps out with expediting feedback since everyone's available.

Team members don't check their messages

  • If this is pretty common or if it's just one person in particular, you need to talk to your team about communication expectations.
  • It also could be just human nature. You ping the team and let them know (or you have a service that does it) and your team just kinda ignores it because they're in the middle of something and they forget about it. Set a reminder for yourself an hour later (or whatever is reasonable) to remind the team that the PR still needs a review. Keep doing it until someone takes responsibility. Be annoying.

The same applies for feedback. Sometimes PRs can be a messy back and forth business. Usually the person who picked up the code review should be the one to respond.

But to answer the question, I found this to be a good flow:

  • Someone puts in a PR. They notify their team. I don't think I've ever heard of other teams checking code, but that's also because teams generally own the code they change to begin with. Cross-team integration is an art of it's own.
  • Team member responds that they'll pick it up. This is usually a "whoever's available, first come, first serve" sort of thing. I've had teams set a rule that you can't pick up new tasks without clearing out reviews. That's kind of a team-by-team sort of basis on how formal you want to be with it.
  • If every thing is good, PR is approved. QA usually approves it during this point as well, but your mileage may vary.
  • Person who puts in the PR to begin with hits the merge button. I know this sounds like a formality, and 95% of the time, it is. Buuuut that 5% there's something that changes between the PR creation and approval and that one last sanity check has saved me at least a dozen critical bugs.
  • Generally this is a day or two long process. If it starts to drag past three, that's asking for trouble and merge headaches.