> forgetting that the reviewer just skimmed it over to get it out of their queue.
There's a difference between cargo cult code reviews and an actual, formal code review.
Beauty is only a nice-to-have feature. If the pretty way is about as fast to produce as the "lunch in the back of your throat" model, sure lets go pretty. But if it's going to cost me 50% of my developer's time, I'll take the quick and dirty every time.
This is gonna start biting you in the ass after a few years.
Woah slow down there soldier. Code reviews are great for many reasons:
1) Introducing new technologies or techniques you used to solve a problem.
2) Letting your styles mesh with your team.
3) Focusing on whether the concern/problem has truely been addressed.
Also, unit testing isn't as widespread as you may think. Many of the projects I've worked on didn't have the infrastructure in place to even start unit testing. Also, ever try to unit test Javascript in the browser, it's possible but it sucks pretty hard.
If you are on a team of 2-3 hardcore devs, then you're probably more correct than not. Thats not usually the case.
2) Use tools to enforce style if you care. Personally, I think it's a waste of time.
3) If you have devs merging code to trunk that isn't fixing the bug and creating the feature requested, then that devs needs replacing.
RE unit testing: I realize it's not that widespread. But anyone developing software and not unit testing is doing it wrong. They should take all the time they waste on code reviews and instead spend it on building out their test suite.
And unit testing Javascript is not only possible, but easy. We do it with Jasmine. It works great. The key is to write good, testable JS instead of the mess of spaghetti that passes for Javascript code in most places.
This is a very good point. If there are very basic things not being done (or done well) focusing on those things is more important than ceremonial code-reviews - and in the beginning it seems code-reviews are often only ceremonial and some shops never get past this stage. I've never seen them work really well aside from cases where they happen organically between two co-workers who like and respect each other.
They don't have to be; in fact, they almost certainly shouldn't be.
Basically unless you trust developers to never be wrong, I'd do reviews. And then if you do trust them, I'd still do reviews to make sure everyone learns what's happening outside of their area and where common utilities can prevent code duplication.
In short - there are things that cannot be realistically automated or simplified into compilation checks. If they don't occur in your environment - great. When they do, look again at code reviews.
For your point about sharing knowledge, your argument works against all forms of sharing knowledge. You may as well say that wikis are unnecessary, because standups, code, comments, and code reviews share knowledge. What you have missed is that these different modes of knowledge sharing work better at sharing different kinds of knowledge, which is why you (should) want all of them.
Also, good developers are not really fungible. Developer X may be really good at making code flow, so that what it is trying to do is not obscured by how it is doing it, and developer Y might have a really good knowledge of algorithms and an eye for optimisation. In that case, having them review each other's code will always result both in better code and in (eventually) better developers.
As far as it goes, I have seen plenty of badly done, useless code reviews. I have also had really well done code reviews, where someone would email me an annotated diff of one of my commits, or where I would pair for a few hours with someone going over some thorny code I or they had committed and making it less thorny, or where we would go back and forth refactoring some piece of code while talking over the whys and wherefores. If I had only had bad code reviews I would be much more sympathetic to the point you are making, but as it is I just think that you have never been involved with a competently done code review.
Not all workplaces are ideal and they will produce less than ideal results. This engenders problems...
(I actually think code reviews are a critical part of a quality culture, but there exist social reasons not to do them)
That's all true but code reviews certainly share a lot of information and feedback specifically related to the context of the day by day development. Which certainly will increase your productivity by teaching you about mistakes and at the same time letting you evangelize someone else in your team/organization. You won't find the same amount of information for your project with as much frequency as you do on code reviews.
[1] http://www.codinghorror.com/blog/2006/01/code-reviews-just-d...
I got rid of them here because I've personally never seen a beneficial affect at any of the places I've worked. Easy to spot bugs still made it to production. Crappy, inelegant code still made it to trunk.
Getting rid of code reviews and making developers personally responsible for what they commit has improved both of those aspects for us.
You might want to check out the book "Code Complete", which says in Chapter 24:
"...the average defect detection rate is only 25 percent for unit testing, 35 percent for function testing, and 45 percent for integration testing. In contrast, the average effectiveness of design and code inspections are 55 and 60 percent."
Some say code reviews prevent bugs. They are wrong. Testing code and static analysis prevent bugs. Require all code to have unit tests and run them automatically. Run static analysis automatically. If you have developers who don't write tests, fire them. Code reviews only give you a false sense of security. You figure: since two people looked at it, it therefore must be good; forgetting that the reviewer just skimmed it over to get it out of their queue.
Some say code reviews enforce style. Assuming you care about style: why not just have your IDE or static analysis tool enforce it for you?
Some say code reviews help share knowledge. That may be true, but it should be unnecessary. Standups share knowledge. Wikis share knowledge. Comments share knowledge. Code should share knowledge. If you have code hitting trunk that every developer on your team can't understand easily and quickly, then you need to start finding new developers.