This week I reviewed my notes about code review and noticed there’s something I haven’t written about: code review often happens too late.
Typically a developer requests code review for their pull request once it’s pretty much done. This is too late.
If you have developed a big change and the reviewer thinks the whole approach is wrong, it’s wasteful to throw the whole change away at this point, and it’s not fun feedback to get. This puts a limit on how big changes the reviewer can ask for in the review.
Another issue is that if in your mind, as the author, the pull request is finished and ready to merge, then it’s not fun when somebody pops up to ask you to make changes. This is especially true if your code review process is slow and you have already moved on to work on something else.1
Dealing with it
How to counteract it? You need to get the reviewer involved more early. Here are a few things that I’ve tried:
Talk about what you’re doing. Telegraph your moves. Tell your colleagues in the daily standup what you’re working on and how you’re going about it. This is simple and usually works well enough.
Split up your work. Do multiple pull requests that build up to the goal you’re working towards. Smaller pull requests are easier to review, too. This has worked well, too, although the overhead in the review process will slow you down.
Request an early review. I’ve sometimes asked my colleagues to review the general approach taken in PR when it’s still a draft. It has not worked very well. Either people do not say anything or then they review the PR as if it was already ready. I still think this could work, but you need to be clear about what kind of feedback you’re looking for.
Bottom line
Code review is an opportunity for collaboration and it works much better if you embrace it as such, both as an author and a reviewer.
Photo: I’ve temporarily ran out of seasonal photos, so here’s a view from summer from top the Wank near Garmisch-Partenkirchen.
-
Starting new work when you’ve got old work in progress is questionable but common. ↩︎