23
Reviewing big pull requests. What, why, how.
In this article, I will try to figure out what leads to big pull requests, what benefits they might provide, what can help you to review them.
First of all, why on earth developers should care about the size of pull requests? In short — it has an enormous number of benefits, and all of them are well described in Google's article: https://google.github.io/eng-practices/review/developer/small-cls.html. To mention some of them:
- Reviewed more quickly
- Reviewed more thoroughly
- Less blocking on reviews
- Easier to design well
and more...
Talking about the size, when pull requests are considered big, it's hard to give a precise number. Often it depends on the type of changes, the project, personal experience. In some cases, 1k changed lines can mean a huge pull request, in others — an acceptable one. Usually, it starts to get harder to review changes when the number of changed files is increasing, rather than the number of changed lines in a couple of files.
Further in the article, a big pull request will mean a relatively large amount of changed files (20+), the number of changed lines doesn't matter.
Is it always possible to make a pull request small enough?
In Google's article, they say it's extremely rare when there is no way to split a pull request into smaller parts. Personally, I'd agree with that, but from my experience, there are some situations when it's almost impossible to split changes, or even it's better to keep them in a single pull request, to name a few:
- A change in a common component of the application. It might be a front-end component, or backend helper class, or whatever else. Such changes usually stretch across dozens of files and it easily turns into a big pull request. It might be possible to split these changes, but reviewing that at once can provide its own benefits: often you can notice a missing set of changes which you won't find reviewing it in parts.
- Small and frequent pull requests are sacrificed for the sake of productivity.
While small pull requests provide a lot of benefits, there is at least one drawback as well: they require more time to create all of them, maintain the stability of the build, and they more often distract colleagues from
watching youtubewhatever they usually do at work. If you're a small team, you want to deliver features faster, you trust each other and have a common vision on how things should be developed — then probably you can skip review at all or review only the final pull request to double-check important things (such as architecture decisions, API design, etc.) from a different perspective. - Poorly established development process. It's either a lack of management experience a team-lead has, a lot of newcomers in the team, or simply you just setting up a team. In such situations, you cannot be sure you won't receive a big pull request on review.
In all cases described above you as a reviewer end up with a big pull request which you have to review. Usually, it's an endless list of changed files which you have to go through, scroll up and down, and make a whole picture of it in your head: why this file has been changed, how the files are interconnected with each other, who's calling this function... It usually hard, takes time, and let's admit that it leads to a shallow review being done if at all.
Just to remind you how it looks like here is an example. It's a pull request of just 16 files:
Where is the best place to start the review from? Always start from the top isn't the best way, isn't it? How to understand what's been done here? Why some file's been changed? Where is a new function invoked?
These are important things to know to perform a high-quality code review, but you don't know them until you read all the changed files and built a full picture in your head.
But what if that picture is built for you automatically? It could be literally a picture where all relations between files are shown and you can understand the scope of changes just after a look and decide where to start.
Sounds cool, right?
That’s what Viezly does.
Viezly is a review tool that splits the changes into smaller independent parts and provides diagrams where the relations between files are shown. With all this, a reviewer can faster get the understanding of what’s been changed and start to review by navigating from one file to another in an optimal way instead of scrolling a list of changes up and down to find out what's going on.
Just compare the example above to this one:
After just a single look at the diagrams, you see what packages have been changed, how the changes are related to each other. You can start reviewing from the root file or the most nested files as you prefer. Furthermore, the changes are divided into standalone groups to make them more comprehensible.
Is that what you trying to keep in mind when reviewing the changes, isn't it?
If you find this useful, as I do, check out more details at viezly.com and stay tuned!
23