Ask HN: Are 1000+ line Code review normal? | Hacker News Reader

Ask HN: Are 1000+ line Code review normal?

1 pointbattery_glasses4 comments

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?

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).

We use a two-level workflow for large refactors like this.

1. create a long-running branch that will eventually be merged to the main branch 2. create N small branches from (1) that get merged back into (1) and contain pieces of the overall effort.

In this way, each smaller change is reviewed and tested along the way and the main branch is unaffected until all work is done.

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!

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.

Reply on news.ycombinator.com