HACKER Q&A
📣 battery_glasses

Are 1000+ line Code review normal?


I recently started working at a new company and one of the other devs routinely submits changes for review that are 1 diff with 1k-5k lines changed.

At my last company the culture was to break up work into easily understood chunks, so the same code would have probably been 5-10 diffs of 100-1000 lines each. I thought it really helped make reviews much more effective and quicker. It took practice to learn how to effectively break up my changes like this but now I can't imagine doing it any other way.

I find it impossible to truly understand whats going on when looking at a set of changes that are bigger than around 1000 lines (not including unit tests.) Apparently people block out 2-3 hour chunks on their calendar to review this person's code.

Is it reasonable to ask this person to break up their work so its easier to review, or am I the outlier and should I just try to get better at reading and reviewing very large diffs?


  👤 AnimalMuppet Accepted Answer ✓
There are changes that can only be made in one giant change. I refactored a monster class into three smaller classes, and it was a huge change. I don't have a line count, but it might have been 5000 lines. Mostly, though, it was moving some things out of the original class and into two new classes. So the diffs were monstrous, but the actual change wasn't that big.

So I've done that... once. I don't think I've done anything like it any other time.

Maybe a way to do this is to say that yes, you can do a code review that large, but only with justification, and the justification has to be accepted by the reviewer, not just by the change author (or the author's manager).


👤 LandR
No, when I worked someplace that did code reviews, if the code review was too large (and 1k lines is far too fucking large), it would just get rejected outright.

The original dev would have to break their work up, it could also mean that the tasks are too large. Generally we would code review each task, and a task would never require 1000 lines of code!


👤 sys_64738
You need a design spec for something that big. You also can't review 5K of lines of code without stepping through it in an editor with tagging enabled.