← Back to context

Comment by SulfurHexaFluri

5 years ago

Not sure if this is common for everyone but I find whenever I get assigned for a review for a monster change, I spend over an hour just working out what the change does and if it seems like it will work. There is no way I could spot tiny potential security exploits in 3000 lines of changed code.

Sending 3,000+ LoC code reviews is not considered good engineering practice. In general it's important to keep code reviews small for the reasons you describe. If it's impossible to make incremental changes in smaller units, that's a sign of an unhealthy code base.

Although it is certainly challenging, you shouldn't spend only an hour reviewing a 3,000 LoC review, unless most of it is trivial refactoring, especially if that code is security-critical and handles untrusted input. That's only 1.2s per line of code. No chance you have good quality control with that amount of attention.

If it's taken someone a whole fortnight to write it, you should expect to spend at least half a day reviewing it, IMO.