My coworker regularly puts up 1500+ line pull requests that are in a single diff. I find it nearly impossible to give good feedback to this many changes at one once - and to go through it thoroughly can take me well over an hour (so I usually am not thorough.)<p>Am I wrong to balk at this? Should I just consider it part of the job and suck it up, or do I have a legitimate grievance here?<p>I've worked at places that would straight up reject a PR if it was too big and demand it be broken out into smaller logical pieces - but we definitely do not have that culture here.
"Acceptable" depends on the team and organization norms and processes, not on the pull request.<p>Reading your post I thought about how often programmers talk about code "readability" and "maintainability," how we should all write our code in a way to make it easy for the next programmer to read and understand. And then I see posts like this, complaining about having to <i>read code</i> and understand and learn from it as part of the job.<p>If you view your job as a programmer as mainly <i>writing more code</i> rather than producing business value you will worry about things like the size of pull requests, number of meetings, learning new tools, even the waste of workplance social interaction. Any time spent not writing code gets devalued and treated like a distracting annoyance.<p>Reading code other people on the team wrote potentially has a lot of value. You can learn more about the overall code base beyond the parts you work on or care about. You might see novel techniques and approaches to solving problems. You might find opportunities to ask questions and collaborate (another thing programmers claim to value but actually don't do that often). Most important, reviewing, understanding, and maybe testing the work of other programmers, in the form of a pull request, directly contributes to the entire project/product in terms of business value, the value that actually matters much more than the isolated silos of code that programmers like to focus on.<p>I sigh when I see a big pull request too, but I remind myself that reviewing and understanding that work ultimately adds more value to the product than any code I will likely write in the time the PR review will take. I will just have less fun and derive less ego gratification from the PR review.
It depends on many factors.
No policy fits every team or situation.
If feedback times - weather due to pipeline or personal/reviews - are long, I would also push larger PRs.
Also reviews rarely really improve anything besides bikeshedding stuff like style or whatever the reviewer might find more pleasant in terms of overall structure.
If reviews were done right you’d have to deep dive into the problem the requester tried to solve and understand his concepts and ideas of the solution.
To do so, I do important reviews (which are super rare) in pair programming style with the requester.
I suggest you should do the same especially if lot of code is changed or added.
I assume you have proper testing in place. Looking at tests can help a lot to understand production code
That depends upon how your organization manages branches, releases, and such.<p>Just take it file by file. If you are using something GitHub Enterprise or the Bitbucket or GitLab equivalents then just leave comments file by file of your progress in reviewing the PR.<p>Is there a parallel stream for corresponding changes to test automation? If so that helps tremendously. Are there rigorous automated lint/validation/conformance rules in place, because if so that helps tremendously. With enough automation in place your review should be a quick look line by line for major violations and questions about why some change was needed from the business requirements perspective as answered by code comments or documentation. 1500 line PRs don't have to be painful if the platform is mature.
You should bring this up during your retrospectives, or at least to the dev whose code you're reviewing, your team lead, and your manager if you don't do retros<p>If the PR is too large, chances are that the task was too big and should have been broken down further. If this happens regularly, your team needs to reevaluate your approach to working on tasks and see how you can organize and ship smaller chunks of code