Hacker News new | ask | show | jobs
by chris_va 3694 days ago
Why do you find them expensive?

I've done thousands in my career. They don't take much time compared to actually writing the code, and adding an extra 5% of engineering time pays major dividends later without drastically reducing throughput.

2 comments

Code review makes you vulnerable to priority inversion. I need this change in now, to meet some deadline, but the reviewer has other priorities. Reviews pile up.

When this happens, a common response is to go work on something else, but this only exacerbates the problem: now you have a bunch of commits awaiting review which are dependent in various ways. If I merge this commit I have to go and fix up that one, and that other one. My work becomes quadratic, nursing a bunch of commits along while waiting their turn through a tiny review pipeline. 5% can be 100%.

This can happen if code reviews are not done well, I agree.

Things can be just as bad if code is not written well.

Doing either well requires good engineering practices and skill. It is part of the profession that you acquire over time, like anything else that can improve development.

Now, sometimes you are at the mercy of a system not filled with really experienced engineers. In such a system, though, I think skipping code reviews for expediency will probably be even worse in the long run.

I was speaking more about the team organization. Code reviews increase in cost as the team becomes more heterogenous and more distributed. If your team consists of a kernel engineer in Paris and an Angular developer in San Francisco, code reviews will be high cost and low value no matter how experienced the devs are.

I agree with you, even in the ideal case of a co-located homogeneous team, it's possible to screw up the process.

Completely agree.
For me continuous integration and refactoring have been the most important practices in keeping a code base clean, robust and agile.

Code review cultures tend to encourage the opposite of these agile practices: monolithic commits, infrequent integration, and minimal diffs -- in other words, practices that tend to result in lesser productivity.

I really like continuous integration/automated tests/code coverage dashboards/automated lint checkers, etc. The more decisions you can cleanly take away, the easier it is to work in a codebase. I agree with you there.

Once you have all that, the purpose of a code review changes a bit, but it is still really useful. Maybe you don't need locking, or maybe you should rewrite the base class, or maybe there is some other part of the code base that should be folded in, or maybe a better interface/algorithm. Stuff like that is hard for a computer to detect for you, at least for the next decade or two :).

The really big thing I push in code reviews is defensive programming. I want to make sure someone in a 2 years, who isn't familiar with the code, will not screw up the codebase after the original authors may have moved on to other things.

How hell does code review encourage those behaviours?

The best commits for code review are small ones. Nobody can be bothered with the big ones.