3、代码审查标准

发表于 更新于

The primary purpose of code review is to make sure that the overall code health of Google’s code base is improving over time. All of the tools and processes of code review are designed to this end.
代码审查（code review）主要是为了确保Google代码库的整体代码质量能随着时间的推移而不断得到改善。代码审查的所有工具和步骤都是为了这个目的而设计的。

In order to accomplish this, a series of trade-offs have to be balanced.
为了达到这个目的，我们必须做一系列的权衡折中。

First, developers must be able to make progress on their tasks. If you never submit an improvement to the codebase, then the codebase never improves. Also, if a reviewer makes it very difficult for any change to go in, then developers are disincentivized to make improvements in the future.
首先，开发人员必须能够在他们自己的（开发）任务上取得进展。如果你从未向代码库提交过代码，那么代码库也不会产生变更。同样，如果评审人的评审标准过于苛刻，那么开发人员也会缺乏足够的动力去提交代码。

On the other hand, it is the duty of the reviewer to make sure that each CL is of such a quality that the overall code health of their codebase is not decreasing as time goes on. This can be tricky, because often, codebases degrade through small decreases in code health over time, especially when a team is under significant time constraints and they feel that they have to take shortcuts in order to accomplish their goals.
另一方面，评审人应确保每次的代码变更都能达到这么一个状态：随着时间推移，代码库的整体质量不会下降。要做到这点可能会有点棘手，因为很多时候，代码库的质量会随着提交上来的代码的小幅质量下降而下降，尤其是当团队在处于时间很紧迫的情况下，为了完成任务，团队成员会觉得他们必须走一些捷径。

Also, a reviewer has ownership and responsibility over the code they are reviewing. They want to ensure that the codebase stays consistent, maintainable, and all of the other things mentioned in “What to look for in a code review.”
同时，评审人对于他们在审核的代码拥有所有权和相应的责任。他们保证代码库的一致性，可维护性，以及《代码审查内容》章节里提到的所以内容。

Thus, we get the following rule as the standard we expect in code reviews:
因此，我们制定了以下规则作为代码审查的标准：

In general, reviewers should favor approving a CL once it is in a state where it definitely improves the overall code health of the system being worked on, even if the CL isn’t perfect.
一般情况下，即使某次代码提交并不是很完美，但是它确实提高了系统的整体代码健康状态，那么评审人就应该通过它。

That is the senior principle among all of the code review guidelines.
这是代码审查指南的最高原则。

There are limitations to this, of course. For example, if a CL adds a feature that the reviewer doesn’t want in their system, then the reviewer can certainly deny approval even if the code is well-designed.
当然，这条原则也有一些局限性。例如，如果某次代码提交添加了一个特性（feature），而该特性却不是评审人所想要加到系统里的，那么毋庸置疑，即使该次代码提交设计得再好，评审人也不会让其通过。

A key point here is that there is no such thing as “perfect” code—there is only better code. Reviewers should not require the author to polish every tiny piece of a CL before granting approval. Rather, the reviewer should balance out the need to make forward progress compared to the importance of the changes they are suggesting. Instead of seeking perfection, what a reviewer should seek is continuous improvement. A CL that, as a whole, improves the maintainability, readability, and understandability of the system shouldn’t be delayed for days or weeks because it isn’t “perfect.”
这里有个比较关键的点，那就是，没有完美的代码，只有更好的代码。评审人不应该要求代码作者在代码评审通过前要对每次代码提交都进行“抛光打蜡”。相反，评审人应该在任务进度和代码建议的重要性之间取得平衡。评审人要追求的是持续改进，而不是一次性的完美。如果某次代码提交提高了系统的可维护性，可读性以及可理解性，那么它就不应该因为不够“完美”而被延迟几天或者几周才通过。

Reviewers should always feel free to leave comments expressing that something could be better, but if it’s not very important, prefix it with something like “Nit: “ to let the author know that it’s just a point of polish that they could choose to ignore.
评审人在认为某些代码可以写得更好时，可以随时发表评论，但如果提出的点并不是非常重要，可以在评论前加个前缀“Nit: ”（Not Important的缩写），通过这个方式让代码作者知道：此处的评论只是一个更好的建议而已，如果选择了忽略，那也是可以的。

Note: Nothing in this document justifies checking in CLs that definitely worsen the overall code health of the system. The only time you would do that would be in an emergency.
注意：本文档中虽然没有内容会证明检查代码提交会使整个系统的代码质量变差。但如果评审人在紧迫的情况下，这种现象也是会出现的。

Mentoring（指导）

Code review can have an important function of teaching developers something new about a language, a framework, or general software design principles. It’s always fine to leave comments that help a developer learn something new. Sharing knowledge is part of improving the code health of a system over time. Just keep in mind that if your comment is purely educational, but not critical to meeting the standards described in this document, prefix it with "Nit: " or otherwise indicate that it’s not mandatory for the author to resolve it in this CL.
代码审查有个很重要的作用，那就是可以传授开发人员有关语言、框架或者通用的软件设计原则方面的一些新东西。审查人员可以随时留下能帮助开发人员学到新东西的评论。分享知识也是提高系统代码质量的方式之一。不过要记住一点，就是，如果你的评论只是纯粹的指导性的，但对于满足本文档里所描述的标准这件事本身来说不重要，那么就应该在评论前面加上“Nit：”或者用别的方式标明，并非强制代码作者去解决该次代码变更的评论里提到的问题。

Principles（原则）

  • Technical facts and data overrule opinions and personal preferences.

  • 技术事实和数据优先于观点和个人偏好。(译注：技术事实是需要一些实践、贸易或科学专业知识才能发现、验证、解释和理解的事实。技术事实和数据一般由公司沉淀而成。此处可以简单理解为做代码审查时产生的观点或者审查人的个人喜欢，应先遵从于公司既有的技术事实和数据。)

  • On matters of style, the style guide is the absolute authority. Any purely style point (whitespace, etc.) that is not in the style guide is a matter of personal preference. The style should be consistent with what is there. If there is no previous style, accept the author’s.

  • 在代码规范问题上，规范规约是绝对权威。 任何不在样式指南中的规范风格（例如空格等）都是个人喜好问题。 代码规范应与现有的规范规约一致。 如果还没有规约，则接受作者的。

  • Aspects of software design are almost never a pure style issue or just a personal preference. They are based on underlying principles and should be weighed on those principles, not simply by personal opinion. Sometimes there are a few valid options. If the author can demonstrate (either through data or based on solid engineering principles) that several approaches are equally valid, then the reviewer should accept the preference of the author. Otherwise the choice is dictated by standard principles of software design.

  • 软件设计的方方面面几乎从来都不是纯粹的规范问题或者只是个人偏好。 它们（指软件设计的各个方面）都基于一些基本原则，应根据这些原则进行权衡，而不仅仅只是个人意见。有时可能会有好几个可行的方案。如果作者能通过数据或者基于可靠的工程设计原则证明这些方案都行之有效，那么审查人员应该接受作者偏向的方案，否则就得给予软件设计的标准原则来做出决定。

  • If no other rule applies, then the reviewer may ask the author to be consistent with what is in the current codebase, as long as that doesn’t worsen the overall code health of the system.

  • 如果没有其它适用的规则，那么审查人员需要要求作者于现有代码库的规则保持一致，因为这有才不会损坏系统的整体代码质量。

Resolving Conflicts（处理冲突，译注：指开发人员和代码审查人员的意见冲突）

In any conflict on a code review, the first step should always be for the developer and reviewer to try to come to consensus, based on the contents of this document and the other documents in The CL Author’s Guide and this Reviewer Guide.
对于代码审查过程中产生的任何意见冲突，开发人员和审查人员第一步应该是尝试基于本文档的内容、变更列表作者指南以及审核人员指南达成一致意见。

When coming to consensus becomes especially difficult, it can help to have a face-to-face meeting or a video conference between the reviewer and the author, instead of just trying to resolve the conflict through code review comments. (If you do this, though, make sure to record the results of the discussion as a comment on the CL, for future readers.)
当很难达成共识时，审查人员和代码作者可以进行面对面沟通或者是视频会议沟通，而不是只是仅通过审查评论来去解决冲突。（但如果确实是这么做的，那么要保证将讨论结果记录到代码变更的评论里，供后面的读者参考。）

If that doesn’t resolve the situation, the most common way to resolve it would be to escalate. Often the escalation path is to a broader team discussion, having a Technical Lead weigh in, asking for a decision from a maintainer of the code, or asking an Eng Manager to help out. Don’t let a CL sit around because the author and the reviewer can’t come to an agreement.
如果通过上述方式还没解决问题，则通常的做法是上升。通常情况下是上升到范围更广的小组讨论，技术leader参与进来，通过询问代码的维护人员来做出决定，或者是让工程经理提供帮助。不要让（有意见冲突的）代码变更放任不管，因为代码作者和审查人员无法达成统一意见。

Next: What to look for in a code review
下一章：代码审查内容