Have you ever wondered why, after applying all of the PR best practices like tiny PRs, with a wall of text description, your pull request review progress and code quality still have not improved? It happens almost everywhere I’ve worked, and an important aspect often gets overlooked.
Let’s explore this with an example. Then, we can break it down into the false miracles of focusing on PR best practices.
Imagine your dream dev team size is around 3–5 headcounts. You’re in an energetic start-up company, and there are quite a lot of features in the roadmap. And an even higher workload for scaling up one, you need to address technical debt as well!
With limited resources and the desire to achieve everything, we often end up running multiple projects simultaneously within a small team. We have several problems with having four engineers working simultaneously on three or four different things. One of the symptoms of this issue is pull requests taking too long to review, or introducing preventable bugs.
And the “usual” approach to solving it is to apply the pull request best practices on the internet that everybody knows. Here I’m telling how it won’t work.
Please keep in mind that your imagination team is having everybody working on different things and they have no time to get to know what others doing.
Smaller PRs
I do agree that the smaller PR size is much easier to review than a big PR, we can review it faster thus can be approved and merged faster. The team is crushing the tickets much quicker. So what’s wrong here?
This bottom-up approach, which I am not really a fan of, has a negative effect in that people will only focus on the small and minor logic and syntax checking. By doing that, the reviewer will be more likely to miss the big picture and the encouragement to try to understand the big picture.
By splitting one big complex flow into 10–20 small PRs and asking the people who have no context and are busy with their tasks, they won’t have time to fully understand other’s work. For PR example, it is like:
-
Add feature flag <- good
-
Add an endpoint <- good
-
Update schema <- good
-
10x PR to Add the splitting logic code and tests from 1 or 2 linear logic flows <- This is where things get crazy.
-
Release feature <- good
There is no way that you can spot the invalid logic in a chain of smaller things, and you have to switch context from a completely different thing unless you spend at least half a day to recall what it is and then half a day to get back to your current work in progress.
Also it will even get worse when the time to review PR is slow. You will have a pile-up of small PRs that depend on the previous PR. The nuclear chain reaction occurs when the first PR gets a change requested
By blindly following the best practices to make it follow stupid rules like PR should have less than 100-200 lines of code or split it whenever possible in terms of deployment, it’s a disaster.
In this case, sometimes, the big PR with a complete flow could be better for this specific situation. I know it’s purely not a good approach or pattern to follow, but hear me out:
-
It forces the reviewer to understand the flow fully, you only need to switch the context less often and may save a lot of time on releasing the feature because we don’t waste time on a lot of the back-and-forth mind switching.
-
By helping the reviewer to get more context and a bigger picture, they can spot the invalid logic or unoptimized parts of the main flow, which is almost impossible when reviewing a fraction of it. Especially when everybody is busy with their work, they will just review some minor syntax, the “nit” improvement, vaguely read the test cases and move on to small PRs.
Then, in fact, your team would spend less time on PR review and deliver much better quality with fewer bugs, also the bonus of context sharing between the team members.
Well, after reading this, you might say:
I’ll get into that now.
“Good PR description” with a wall of text
It’s somewhat funny to me that people believe that people who are too busy with their current work would have time to read a wall of text in an unimportant and non-related work from somebody’s pull request.
I don’t think a fast-paced, moving team should have that long description in the daily PR review process. The long PR description indicates that you’re trying to explain your work to others who don’t know what you’re working on. This means the target audiences should be wider stakeholders, and the PR should focus on the general architecture flow or the main business stream.
Good example:
-
The architecture decision to use a push instead of pull approach for pub-sub is what all engineers should pay attention to in the current WIP.
-
Hardcore engineer work/optimization that can only be explained by nerdy terms and numbers and is not relevant to the team’s feature work, e.g., improving BTC block hash calculation, fixing the deep life cycle handler bugs,…
-
Refactor the main biz flow to use A instead of B in the feature X team.
-
Solo one-off tickets in general.
-
…
Bad example to put a wall of text:
- Add tests for 1 out of 10 schemas tiny logic check in a straightforward API to update the user profile.
And it’s not for the small progress from your tiny feature. We already have the ticket’s description and project planning for it! It’s like bringing a bazooka to a knife fight.
Conclusion
Pull request best practices or any other best practices are often seen as solutions as the first choices. They aren’t always the answer to deeper issues within a team. I’m not encouraging people to move away from best practices. If it can be small, great, the same for detailed PR description. But that should not be the priority and the sole solution.
To me, it’s a red flag when a lead or manager repeats well-known advice without taking the time to investigate and deeply understand the specific issues facing the team.
Ultimately, it’s not just about sticking to the rules but about building a workflow that helps your team collaborate more effectively. By stepping back to see the bigger picture and tackling the root cause of inefficiencies, you’ll notice that the quality of your code — and the value of your reviews — will improve on their own.