Disclaimer: There are some similar questions, but I didn't find any which touch specifically the problems you face while reviewing a large pull request.
Problem
I feel my code reviews could be done in a better way. I'm particularly talking about big code reviews with many changes across 20+ files.
It's pretty simple to catch obvious local code problems. Understanding whether the code meets business criteria is a different story though.
I have troubles following the thought process of the code author. It's pretty hard when the changes are numerous and spread across multiple files. I try to focus on the groups of files related to a particular piece of changes. Then review the groups one by one. Unfortunately the tool I use (Atlassian Bitbucket) is not very helpful. Whenever I visit a file, it gets marked as seen, even though it often turns out not to be related to the currently examined piece of changes. Not to mention that some files should be visited multiple times and their changes reviewed piece-by-piece. Also coming back to relevant files when you follow a bad path isn't easy.
Possible solutions, and why they don't work for me
Reviewing a pull request by commits often solves the size problems, but I don't like it since I'll frequently be looking at outdated changes.
Of course, creating smaller pull requests seems like a remedy, but it is what it is, sometimes you get a large pull request and it has to be reviewed.
You can also ignore logical aspect of the code as a whole, but it seems pretty risky, particularly when the code comes from an inexperienced programmer.
Using a better tool could be helpful, but I didn't find one.
Questions
- Do you have similar problems with your code reviews? How do you face them?
- Maybe you have better tools?
Best Answer
We had these problems and asking the question below has been working well for us:
Does the PR do one thing that can be merged and can be independently tested?
We try to break PRs by single responsibility (SR). After initial push back folks were surprised to find that even something small albeit single can be large.
The SR makes it really easy to review and also disseminates knowledge of the expected implementation.
This also allows for incremental refactors as more is added and PR turnaround time is drastically reduced!
I’d suggest breaking them up by SR if possible and see if it works for you. Takes some practice to do it that way.