What do you do when code review is just too hard

code-reviewsdevelopment-process

OK so a lot of code review is fairly routine. But occasionally there are changes that broadly impact existing complex, fragile code. In this situation, the amount of time it would take to verify the safety of the changes, absence of regression, etc. is excessive. Perhaps even exceeding the time it took to do the development itself.

What to do in this situation? Merge and hope nothing slips through? (Not advocating that!) Do the best one can and try only to spot any obvious flaws (perhaps this is the most code review should aim for anyway?) Merge and test extra-thoroughly as a better alternative than code review at all?

This is not specifically a question whether testing should be done as part of a code review. This is a question asking what the best options are in the situation as described, especially with a pressing deadline, no comprehensive suite of unit tests available or unit tests not viable for the fragmented code that's changed.

EDIT: I get the impression that a few of the answers/comments so far have picked up on my phrase "broadly impact", and possibly taken that to mean that the change involved a large number of lines of code. I can understand this being the interpretation, but that wasn't really my intention. By "broadly impact", I mean for example the potential for regression is high because of the interconnectedness of the codebase, or the scope of knock-on effects, not necessarily that the change itself is a large one. For example, a developer might find a way to fix a bug with a single line by calling an existing high level routine that cascades calls to many lower level routines. Testing and verifying that the bug fix worked is easy. Manually validating (via code review) the impact of all the knock-on effects is much more difficult.

Best Answer

The premise of the question is, frankly, astounding. We suppose that there is a large change to fragile, complex code, and that there is simply not enough time to review it properly. This is the very last code you should be spending less time on reviewing! This question indicates that you have structural problems not only in your code itself, but in your methodology of managing change.

So how to deal with this situation? Start by not getting into it in the first place:

  • Identify sources of complexity, and apply careful, heavily reviewed, correct refactorings to increase the level of abstraction. The code should be understandable by a fresh-out-of-college new employee who knows something about your business domain.

  • Identify sources of fragility; this could be by review of the code itself, by examining the history of bug fixes to the code, and so on. Determine which subsystems are fragile and make them more robust. Add debugging logic. Add assertions. Create a slow but obviously correct implementation of the same algorithm and in your debug build, run both and verify that they agree. In your debug build, cause rare situations to occur more frequently. (For example, make a memory allocator that always moves a block on reallocation, or always allocates a block at the end of a page, or whatever.) Make the code robust in the face of changes to its context. Now you don't have fragile code anymore; now you have code that finds the bugs, rather than causes the bugs.

  • Write a suite of automated tests. Obviously.

  • Don't make broad changes. Make a series of small, targeted changes, each of which can be seen to be correct.

But fundamentally, your scenario is "we have dug ourselves into a hole of technical debt and each complex, unreviewed change digs us deeper; what should we do?". What do you do when you find yourself in that hole? Stop digging. If you have so much debt that you are unable to do basic tasks like reviewing each other's code then you need to stop making more debt and spend time paying it off.