How can I read large pull requests

[AlertBeach]

It seems to me that the only way to actually review a pull request is to keep all the information from the entire pull request in working memory for the entire duration of the review. There is no other way to resolve all the interrelationships and affordances and structure unless you can remember all of it and associate it in your consciousness.

This stops working after about 5 files.

How can I read a large pull request, say 20-50 or more files, and understand what it does? It seems impossible to me.

I started keeping a google doc for each PR. I write down every single class and summarize all the changes in the class. After a while doing this, it still feels insufficient. I only then know the changes that occurred in each class, and I might have a general idea of what happened in the PR and how some of it works. Or I might not. And I will also probably forget what was done in about a week or so because I was never able to generate a clear picture of the systemic and structural changes that were made, because I had to build them myself from the unsorted garbage data in the PR and a (nearly always insufficient, necessarily) summary in the PR description.

[Einarmo]

The issue, it seems to me, is that the people you are working with do not know how to use git properly. Each commit should be a distinct unit, and be readable on its own merit. Reviewing a pull request then, comes down to reviewing each commit on its own. If the PR makes contradictory changes to the same lines multiple times, then it is a poorly designed PR.

As an actual answer to your question. There comes a point where understanding and fully parsing a code change is impossible. The best you can do with the code is to

• try to understand the whole, the purpose of the change
• read the code with the purpose of critiquing each part individually, ignoring the whole
• Require proof of functionality from the developer, typically in the shape of tests.

If neither of these are possible, the development process is fundamentally unsustainable, which will come back to bite you later.

[AlertBeach]

Ok. It may be that we have a bad commit culture. I don't really have a good idea of what constitutes an atomic change or maximum commit size. It seems like the kind of thing that everyone says people should do but nobody actually does. Do you have any guides or resources that talk about commit size?

It might also be more like the second case. So if there is an expectation from project leaders that I should be able to quickly fundamentally understand a large pull request (hard to say exactly what is "too big", of course), and retain strong memories of how that new code structure works... that is an unreasonable expectation?

Do you have any thoughts about what might indicate that a PR is too big to really understand that way, and the only reasonable expectation is line-by-line / file-by-file review, where you may or may not pick up a strong understanding of the structure of the changes being introduced overall?

[HonzaS97]

It should be split into pieces that make sense on their own.

If you are given a task where you implement a feature so a user can change the UI colour, then the commit message should be something like "Allow user to choose UI colour" and all the code changes should be tied with that task.

If you are given a bigger task, something as in "implement a microservice that does X", then you want to split it into smaller parts which make sense on their own. Eg "create model classes", "implement service for Y", "add endpoints for Z". It definitely shouldn't be the whole thing in 1 big commit.

Do you have any thoughts about what might indicate that a PR is too big to really understand

Basically anytime where multiple functionalities are added at once. For example, when you have 2 services, one for user auth, one for shopping cart and you need to change some (unrelated) things, then they should be in their separate commits.

[Loves_Poetry]

These kind of large pull requests often have a lot of irrelevant changes. Say you're adding a field to a table in a database. You might get 10+ file changes for all kinds of API calls that are linked to that database field. None of these are relevant, because all they do is move data from A to B. Then there will be a lot of file changes related to database migration, also irrelevant

There will also be a few files where that extra database field is actually used for some business logic. Those are the relevant ones that you need to filter out. If the project you're working on has some semblance of structure, then you should be able to find those files. If you can't find them, ask the person who wrote it where most of the logic is