谷歌是如何做代码审查的

Things Everyone Should Do: Code Review
Jul 06 2011 Published by MarkCC under Programming
As I alluded to in my last post (which I will be correcting shortly), I no longer work for Google. I still haven't decided quite where I'm going to wind up - I've got a couple of excellent offers to choose between. But in the interim, since I'm not technically employed by anyone, I thought I'd do a bit of writing about some professional things that are interesting, but that might have caused tension with coworkers or management.
Google is a really cool company. And they've done some really amazing things - both outside the company, where users can see it, and inside the company. There are a couple of things about the inside that aren't confidential, but which also haven't been discussed all that widely on the outside. That's what I want to talk about.
The biggest thing that makes Google's code so good is simple: code review. That's not specific to Google - it's widely recognized as a good idea, and a lot of people do it. But I've never seen another large company where it was such a universal. At Google, no code, for any product, for any project, gets checked in until it gets a positive review.
Everyone should do this. And I don't just mean informally: this should really be a universal rule of serious software development. Not just product code - everything. It's not that much work, and it makes a huge difference.
What do you get out of code review?
There's the obvious: having a second set of eyes look over code before it gets checked in catches bugs. This is the most widely cited, widely recognized benefit of code review. But in my experience, it's the least valuable one. People do find bugs in code review. But the overwhelming majority of bugs that are caught in code review are, frankly, trivial bugs which would have taken the author a couple of minutes to find. The bugs that actually take time to find don't get caught in review.
The biggest advantage of code review is purely social. If you're programming and you know that your coworkers are going to look at your code, you program differently. You'll write code that's neater, better documented, and better organized -- because you'll know that people who's opinions you care about will be looking at your code. Without review, you know that people will look at code eventually. But because it's not immediate, it doesn't have the same sense of urgency, and it doesn't have the same feeling of personal judgement.
There's one more big benefit. Code reviews spread knowledge. In a lot of development groups, each person has a core component that they're responsible for, and each person is very focused on their own component. As long as their coworkers components don't break their code, they don't look at it. The effect of this is that for each component, only one person has any familiarity with the code. If that person takes time off or - god forbid - leaves the company, no one knows anything about it. With code review, you have at least two people who are familiar with code - the author, and the reviewer. The reviewer doesn't know as much about the code as the author - but they're familiar with the design and the structure of it, which is incredibly valuable.
Of course, nothing is every completely simple. From my experience, it takes some time before you get good at reviewing code. There are some pitfalls that I've seen that cause a lot of trouble - and since they come up particularly frequently among inexperienced reviewers, they give people trying code reviews a bad experience, and so become a major barrier to adopting code review as a practice.
The biggest rule is that the point of code review is to find problems in code before it gets committed - what you're looking for is correctness. The most common mistake in code review - the mistake that everyone makes when they're new to it - is judging code by whether it's what the reviewer would have written.
Given a problem, there are usually a dozen different ways to solve it. Andgiven a solution, there's a million ways to render it as code. As a reviewer, your job isn't to make sure that the code is what you would have written - because it won't be. Your job as a reviewer of a piece of code is to make sure that the code as written by its author is correct. When this rule gets broken, you end up with hard feelings and frustration all around - which isn't a good thing.
The thing is, this is such a thoroughly natural mistake to make. If you're a programmer, when you look at a problem, you can see a solution - and you think of what you've seen as the solution. But it isn't - and to be a good reviewer, you need to get that.
The second major pitfall of review is that people feel obligated to say something. You know that the author spent a lot of time and effort working on the code - shouldn't you say something?
No, you shouldn't.
There is never anything wrong with just saying "Yup, looks good". If you constantly go hunting to try to find something to criticize, then all that you accomplish is to wreck your own credibility. When you repeatedly make things to criticize just to find something to say, then the people who's code you review will learn that when you say something, that you're just saying it to fill the silence. Your comments won't be taken seriously.
Third is speed. You shouldn't rush through a code review - but also, you need to do it promptly. Your coworkers are waiting for you. If you and your coworkers aren't willing to take the time to get reviews done, and done quickly, then people are going to get frustrated, and code review is just going to cause frustration. It may seem like it's an interruption to drop things to do a review. It shouldn't be. You don't need to drop everything the moment someone asks you to do a review. But within a couple of hours, you will take a break from what you're doing - to get a drink, to go to the bathroom, to talk a walk. When you get back from that, you can do the review and get it done. If you do, then no one will every be left hanging for a long time waiting on you.
在上一篇文章中提到过，我已经不在Google工作了。我还没有想清楚应该去哪里—有两三个非常好的工作机会摆在我面前。因为在这段做决定时间里，我不再受雇于任何人，我想可以写一些专业性的东西，一些很有趣，但也会在同事和管理工作中导致关系紧张的东西。
Google是一个非常优秀的公司。他们做出了很多令人称赞的东西—既是公司外部，人们可以看到的东西，也是公司内部。有一些在公司内部并不属于保密的事情，在外部并没有给予足够广泛的讨论。这就是我今天要说的。
让Google的程序如此优秀的一个最重要的事情看起来是非常的简单：代码审查。并不是只有Google做这个事情—代码审查已经被广泛的认可为一种非常好的做法，很多人都在这样做。但我还没有看到第二家这样大的公司能把这种事情运用的如此普遍。在Google，没有程序，任何产品、任何项目的程序代码，可以在没有经过有效的代码审查前提交到代码库里的。
所有人都要经过代码审查。并且很正规的：这种事情应该成为任何重要的软件开发工作中一个基本制度。并不单指产品程序——所有东西。它不需要很多的工作，但它的效果是巨大的。
从代码审查里能得到什么？
很显然：在代码提交前，用第二群眼睛检查一遍，防止bug混入。这是对其最常见的理解，是对代码审查的好处的最广泛的认识。但是，依我的经验来看，这反倒是它最不重要的一点。人们确实在代码审查中找到了bug。可是，这些在代码审查中能发现的绝大部分bug，很显然，都是微不足道的bug，程序的作者花几分钟的时间就能发现它们。真正需要花时间去发现的bug不是在代码审查里能找到的。
代码审查的最大的功用是纯社会性的。如果你在编程，而且知道将会有同事检查你的代码，你编程态度就完全不一样了。你写出的代码将更加整洁，有更好的注释，更好的程序结构——因为你知道，那个你很在意的人将会查看你的程序。没有代码审查，你知道人们最终还是会看你的程序。但这种事情不是立即发生的事，它不会给你带来同等的紧迫感，它不会给你相同的个人评判的那种感受。
还有一个非常重要的好处。代码审查能传播知识。在很多的开发团队里，经常每一个人负责一个核心模块，每个人都只关注他自己的那个模块。除非是同事的模块影响了自己的程序，他们从不相互交流。这种情况的后果是，每个模块只有一个人熟悉里面的代码。如果这个人休假或——但愿不是——辞职了，其他人则束手无策。通过代码审查，至少会有两个人熟悉这些程序——作者，以及审查者。审查者并不能像程序的作者一样对程序十分了解——但他会熟悉程序的设计和架构，这是极其重要的。
当然，没有什么事情能简单的做下来的。依我的经验，在你能正确的进行代码审查前，你需要花时间锻炼学习。我发现人们在代码审查时经常会犯一些错误，导致不少麻烦——尤其在一些缺乏经验的审查者中经常的出现，他们给了人们一个很遭的代码审查的体验，成为了人们接受代码审查制度的一个障碍。
最重要的一个原则：代码审查用意是在代码提交前找到其中的问题——你要发现是它的正确。在代码审查中最常犯的错误——几乎每个新手都会犯的错误——是，审查者根据自己的编程习惯来评判别人的代码。
对于一个问题，通常我们能找出十几种方法去解决。对于一种解决方案，我们能有百万种编码方案来实现它。作为一个审查者，你的任务不是来确保被审查的代码都采用的是你的编码风格——因为它不可能跟你写的一样。作为一段代码的审查者的任务是确保由作者自己写出的代码是正确的。一旦这个原则被打破，你最终将会倍感折磨，深受挫折——这可不是我们想要的结果。
问题在于，这种错误是如此的普遍而易犯。如果你是个程序员，当你遇到一个问题，你能想到一种解决方案——你就把你想到的方案作为标准答案。但事情不是这样的——作为一个好的审查者，你需要明白这个道理。
代码审查的第二个易犯的毛病是，人们觉得有压力，感觉非要说点什么才好。你知道作者用了大量的时间和精力来实现这些程序——不该说点什么吗？
不，你不需要。
只说一句“哇，不错呀”，任何时候都不会不合适。如果你总是力图找出一点什么东西来批评，你这样做的结果只会损害自己的威望。当你不厌其烦的找出一些东西来，只是为了说些什么，被审查人就会知道，你说这些话只是为了填补寂静。你的评论将不再被人重视。
第三是速度。你不能匆匆忙忙的进行一次代码审查——但你也要能迅速的完成。你的同伴在等你。如果你和你的同事并不想花太多时间进行代码复查，你们很快的完成，那被审查者会觉得很沮丧，这种代码审查带来的只有失望的感觉。就好象是打搅了大家，使大家放下手头的工作来进行审查。事情不该是这样。你并不需要推掉手头上的任何事情来做代码审查。但如果中途耽误了几个小时，你中间还要休息一会，喝杯茶，冲个澡，或谈会儿闲话。当你回到审查现场，你可以继续下去，把事情做完。如果你真是这样，我想没有愿意在那干等着你。