如何有效地执行代码审查

by Sandor Dargo

由Sandor Dargo

使用这些有效的代码审查指南创建合理的办公环境 (Create a sane office environment with these effective code review guidelines)

In my new team, we are working on several guidelines, rules and process improvements. Why do we think these are so important? If things are well-documented, it’s easier for a newcomer to start delivering value. It reduces the possibilities of errors for everyone. It removes lots of possibilities for arguments. And we all know that one cannot win an argument, so we should avoid them at all costs.

在我的新团队中，我们正在研究一些准则，规则和流程改进。 为什么我们认为这些是如此重要？ 如果一切都有据可查，那么新手开始创造价值就更容易了。 它减少了每个人出错的可能性。 它消除了很多争论的可能性。 我们都知道， 不能赢得一场辩论 ，因此我们应不惜一切代价避免他们。

For a more detail vision about the importance of guidelines, please check out this article. I’ll revisit it soon, by the way.

要详细了解指南的重要性，请查看本文 。 顺便说一下，我会尽快重温。

This time, I’m going to focus on code reviews and on the corresponding guidelines.

这次，我将专注于代码审查和相应的准则。

代码审查的目的 (The aim of the code review)

Reviewing a pull request is an important and sensitive task. In my opinion, it is at least as important as writing the code. Besides, reviewing someone else’s code is a not just a technical task, it’s also a human one. That gives most of its delicateness.

审核拉取请求是一项重要而敏感的任务。 在我看来，它至少与编写代码一样重要。 此外，审查他人的代码不仅是一项技术任务，而且是一项人工任务。 这充分体现了其精致性。

So let me start with the most important rule that you should always have in mind whenever you start to review a pull request or whenever you open up a review you received:

因此，让我从最重要的规则开始，无论何时开始审阅拉取请求或何时打开收到的评论，都应始终牢记：

No comment should be personal. No comment should be made about the author or the reviewer. A review must always be about the code!

没有评论应该是个人的。 不要对作者或审稿人发表评论。 审查必须始终与代码有关！

The aim of a code review is to make the code better, to detect bugs before merging and delivering, and to improve maintainability of a given code base.

代码审查的目的是使代码更好，在合并和交付之前检测错误，并提高给定代码库的可维护性。

代码审核中要检查的项目 (Items to be checked in a code review)

Reviewing code is difficult , and it’s a very broad task. According to my bosses, I’m considered a good code reviewer. But still, I think my effectiveness could be improved a lot. I think that following checklists, in most cases, can be a huge help.

审查代码很困难，这是一项非常广泛的任务。 根据老板的说法，我被认为是一名优秀的代码审阅者。 但是，我仍然认为我的效率可以大大提高。 我认为，在大多数情况下，遵循清单可以提供巨大帮助。

Now, obviously, some of those checklists and/or tasks will be language-specific. However, reviews are helped by the same concepts existing in multiple code languages.

现在，显然，其中一些清单和/或任务将是特定于语言的。 但是，通过多种代码语言中存在的相同概念可以帮助进行审阅。

These lists are mostly here to give you some ideas, as they are far from complete. Feel free to use them, update them, personalize them, or just let them inspire you to come up with completely new ones.

这些清单大多是在这里为您提供一些想法，因为它们还远远不够完整。 随意使用它们，对其进行更新，对其进行个性化设置，或者只是让它们激发您提出全新的想法。

I think that one reviewer shouldn’t use them all, but maybe just a few. But if you have separate checklists, it’s easy to share the tasks.

我认为，一位审稿人不应该全部使用它们，而应该只使用其中一些。 但是，如果您有单独的清单，则共享任务很容易。

Not all the checklists are there to be used for all code reviews. If the pull request is a really small bugfix, just correcting an off-by-one in a condition, it will not require checking the design of the whole domain.

并非所有清单都可用于所有代码审查。 如果拉取请求是一个很小的错误修正，只需在某种情况下更正单个错误，就不需要检查整个域的设计。

清单类型 (Types of checklists)

全过程清单 (Full process checklist)

This one focuses on some foundational characteristics of a pull request. Make sure that the new commits don’t break the compilation or the tests. Your Continuous Integration pipeline should take care of this, but in case not — don’t forget about it. Otherwise, check these:

这一节重点介绍拉取请求的一些基本特征。 确保新提交不会破坏编译或测试。 您的持续集成管道应解决此问题，但如果不这样做，请不要忘记。 否则，请检查以下内容：

• Are new unit/regression tests added?是否添加了新的单元/回归测试？
• Are there new compiler warnings?是否有新的编译器警告？
• Does the change functionally make sense?更改在功能上有意义吗？
• Are there a lot of dependencies?有很多依赖性吗？
• Are the commit messages clean?提交消息是否干净？

SOLID(面向对象设计)原理清单 (SOLID (object-oriented design) principles checklist)

In order to verify the sanity of the design, it’s worth going through the SOLID principles. It’s useful to expand these items into sublists, which helps to verify each principle:

为了验证设计的合理性，值得遵循SOLID原则。 将这些项目扩展到子列表中很有用，这有助于验证每个原理：

• Single responsibility principles单一责任原则
• Open/closed principle开闭原则
• Liskov substitution principle里斯科夫替代原则
• Interface segregation principle接口隔离原理
• Dependency inversion principle依赖倒置原则

安全清单 (Security checklist)

Your application might or might not be security-critical. As soon as it’s hacked once or it fails because of some messy input, it will become one. This checklist should be heavily language dependent (I’m giving you one for C++). The list is extracted mainly from this talk on secure programming practices at the NDC Security Conference at 2018

您的应用程序可能会或可能不会对安全性至关重要。 一旦它被黑客入侵了一次，或者由于一些混乱的输入而失败了，它将变成一个。 此清单应在很大程度上取决于语言(我为您提供了C ++)。 该列表主要摘自2018年NDC安全会议上有关安全编程实践的演讲

• Is external input handled properly?是否正确处理了外部输入？
• Are C-style interfaces used?是否使用C风格的接口？

• Is the new operator superfluously used instead of stack allocation?

  是否多余地使用了new运算符而不是堆栈分配？

• Are there lots of (error-prone) size calculations?有很多(容易出错的)大小计算吗？
• Are pointers used a lot?指针经常使用吗？
• Are shared_ptrs used a lot?经常使用shared_ptrs吗？
• Are there any threads?有线程吗？

测试最佳做法清单 (Testing best practices checklist)

I hope we all agree that testing is part of a developer’s job. If we had a discussion on testing, it would be about the different ways of doing it, not whether we should do it or not.

我希望我们都同意测试是开发人员工作的一部分。 如果我们讨论测试，那就是测试的不同方式，而不是我们是否应该进行测试。

The bad news is that there is no one way fits for all. Still, I’d advise you to follow the cycle of Test Driven Development. The good news is that, on a project, there is at least a common understanding about what should be done.

坏消息是，没有一种方法适合所有人。 不过，我还是建议您遵循“测试驱动开发”的周期。 好消息是，在一个项目上，至少应该达成共识。

If there is none, step in and advocate for testing, gather articles and studies, and convince the team. You’ll be much more respected.

如果没有，请介入并主张进行测试，收集文章和研究，并说服团队。 您将受到更多的尊重。

Here a few points to clarify in regards to the testing part:

以下是有关测试部分的一些要点：

• Are there enough unit tests?有足够的单元测试吗？
• Are there enough non-regression tests?有足够的非回归测试吗？
• Do tests test one thing?测试测试一件事吗？
• Do they have assertions? (A test might have multiple assertions, still logically they assert one thing)他们有主张吗？ (一个测试可能有多个断言，但从逻辑上讲，它们断言了一件事)
• Are they readable?它们可读吗？
• How are dates used? (Fixed vs. generated)如何使用日期？ (固定与生成)

代码可读性清单 (Code readability checklist)

We — developers — are all authors. If we do an impeccable job, our code will read like a prose. I’m not saying that you should always reach this goal for the whole codebase, but you should aim for that.

我们(开发人员)都是作者。 如果我们做的无懈可击， 我们的代码将像散文一样读起来 。 我并不是说您应该始终在整个代码库中都达到这个目标，但是您应该为此而努力。

The code reviewer has a huge responsibility here. If you are reading a pull request, please think about the following questions:

代码审查员在这里负有巨大责任。 如果您正在阅读请求请求，请考虑以下问题：

• Are names meaningful?名称有意义吗？
• Are classes/functions small enough?类/函数足够小吗？
• Does the code “read like a prose”?该代码是否“读起来像散文”？
• Is the code well-formatted?代码格式正确吗？
• Is there duplicated code?有重复的代码吗？

资源处理清单，又称RAII (Resource handling checklist, a.k.a. RAII)

This last one is rather language-specific. It’s not only for C++, but mostly. If you are a C++ developer and you’ve ever fought against dangling pointers, memory leaks, and nasty core dumps, then you know what I mean.

最后一个是特定于语言的。 它不仅适用于C ++，而且适用于大多数情况。 如果您是C ++开发人员，并且曾经与悬空指针，内存泄漏和讨厌的内核转储进行过斗争，那么您就知道我的意思了。

It can be really difficult for a non-expert to spot these issues. But following a helpful checklist can help you both in pointing out the problematic lines and in developing the RAII expertise.

非专家真的很难发现这些问题。 但是遵循一份有用的清单可以帮助您指出有问题的路线以及发展RAII的专业知识。

• Is object ownership clarified?对象所有权是否明确？
• Are objects properly destroyed/ is the memory correctly deallocated?对象是否已正确销毁/内存是否已正确释放？
• Are new fields properly handled?是否正确处理了新字段？
• Are Fields correctly initialized in the constructors?字段是否在构造函数中正确初始化？
• Are comparison operators updated?比较运算符是否已更新？

准则审查者的行为准则 (The code of conduct for code reviewers)

As stated before, commenting on someone else’s code is also a human task, so be nice to your fellow developers. Here are some pieces of advice. Following them will markedly decrease the chance that developers will cry or throw chairs at each other in the office. (But I have never seen the latter — so far…)

如前所述，注释其他人的代码也是一项艰巨的任务，因此对其他开发人员要好。 这里有一些建议。 跟随他们将显着减少开发人员在办公室互相哭泣或丢椅子的机会。 (但到目前为止，我还没有见过后者-)

不要 (Don’ts)

• Don’t refer to personal traits and don’t judge (for instance, refrain from saying you/your code is stupid…)不要提及个人特质，不要做出判断(例如，不要说您/您的代码很愚蠢……)
• Don’t make demands (at least put a please in there and explain why you’re asking for a change)不要提出要求(至少在里面请一个，并解释为什么您要更改)
• Don’t be sarcastic, even if you are buddies. Other reviewers/readers might find some comments inappropriate即使您是哥们，也不要讽刺。 其他评论者/读者可能会发现一些不适当的评论
• Never say never, nor always. There will always be exceptions. So treat this rule with care…永远不要说永远也永远不要。 总是会有例外。 因此，请谨慎对待此规则...
• Avoid selective ownership of the code (that is, don’t use “mine,” “not mine,” “yours”…)避免对代码进行有选择的所有权(即，不要使用“我的”，“不是我的”，“您的……”)

多斯 (Dos)

• Ask questions.问问题。
• Ask for clarification.要求澄清。
• Be explicit. Remember people don’t always understand your intentions online.要明确。 请记住，人们并不总是在线上了解您的意图。
• Seek to understand the author’s perspective.试图了解作者的观点。
• If discussions turn too philosophical or academic, move the discussion offline如果讨论太过哲学或学术性问题，请离线进行讨论
• Identify ways to simplify the code while still solving the problem.确定在解决问题的同时简化代码的方法。
• Communicate which ideas you feel strongly about and which you don’t. If you just express your preference, say that it’s only your preference.传达您对哪些想法有强烈的想法，而哪些则不是。 如果您只是表达自己的偏好，请说这只是您的偏好。
• Educate. If you suggest something, share proofs for why it’s better (like articles, studies, books, and so on).教育。 如果您提出建议，请分享证明它为何更好的证据(例如文章，研究，书籍等)。

作者规则 (Rules for the authors)

• Be humble and honest about the submitted code. Mistakes happen every day, and the process is there to support you.对提交的代码保持谦虚和诚实。 每天都会发生错误，并且流程会一直为您提供支持。
• Remember that you shouldn’t take it personally. The review is of the code, not of you.请记住，您不应该个人化。 审阅的是代码，而不是您的。
• Explain why the code exists.说明代码为何存在。
• Follow guidelines.遵循准则。
• Seek to understand the reviewer’s perspective.试图了解审稿人的观点。
• Be grateful for alternative suggestions and keep the discussion technical. Try to learn from different perspectives.感谢其他建议，并保持讨论的技术性。 尝试从不同角度学习。

呼吁采取行动 (Call to action)

• Make thorough code reviews. You will learn a lot, just like your fellow developers.进行彻底的代码审查。 您将像其他开发人员一样学到很多东西。
• Emphasize the importance of proper code reviews in your teams, and if necessary, educate your colleagues how to review code.强调在团队中进行正确的代码审查的重要性，并在必要时教育同事如何审查代码。

• Check out and star this repository where I’ve collected some checklists and ideas. Feel free to contribute, and add what you have found important!

  签出并在该资料库中加注星标，以收集我的一些清单和想法。 随时做出贡献，并添加您发现的重要内容！

This article has been originally published on my blog.

本文最初发布在我的博客上 。

翻译自: https://www.freecodecamp.org/news/create-a-sane-office-environment-with-these-effective-code-review-guidelines-1d99ae2bdd47/

如何有效地执行代码审查