作者 | Elena Flat

译者 | 弯月，责编 | 屠敏

出品 | CSDN（ID：CSDNnews）

假设有这样一种情况：

产品负责人B在Slack上@了三位开发人员：A、C和D，通知他们紧急修复某个问题。第二天上午10点有一个客户会议，所以他们必须马上完成修改，B才能在会上进行演示。现在是下午六点。A正在下班回家的路上，而C在前往度假的路上（她请了假）。但是，D在办公桌前。

接下来会发生什么：

• D看到了B的消息，他告诉B，没问题，他可以修复这个问题。但是，因为他家里有一点事儿，所以晚上9点以后以及明天整个上午他都没法工作。因此，他来不及将修改推上线，也无法在明天早上的会议中提供支持。

• C在车上看到了Slack上的对话，她说：“没问题，如果有需要的话，明天早上我可以参加会议。”

• D开始编写了一段代码，然后建立了一个PR。

• 晚上8点，A到家后看到了Slack上的消息，她告诉团队的其他成员她可以帮忙。她打开了D的PR，发现由于时间紧迫，D的代码没有经过测试，于是她决定添加测试。由于功能要求在Slack上，而且他们可以在彼此的分支上进行协作，所以这不是问题。

• 晚上9点，D下班了。

• 晚上10点，A完成了测试，此时C上线了，她可以审查代码。C发现了一个错误（由于当时很困，所以A误解了一些功能要求），在A调整好代码后，他们执行了实时发布。

• 第二天早上，由于B在最后一刻临时提了一些额外的要求，所以C做了一些更改，并完成了实时发布。D从手机上批准了第二个PR（他由于担心，忍不住上去检查了一下）。最后客户会议取得了巨大成功。

其实，这是一件真事，就发生在上周四。我为我们的团队感到非常自豪。以前，我在一家创业公司工作的时候，也曾因为通宵加班而感到自豪：我的工作如此重要，如果我不24小时加班的话，地球就不转了。当时的这种自豪感很愚蠢，但这次的自豪感则完全不同。

这次，我的自豪源自不同的因素：

• 我们团队中的三位开发都非常熟悉代码，而且具备了修改代码和审查代码的能力。

• 所有人都在紧急时刻挺身而出，没有让某一个人孤身彻夜奋战。

• 只要我们三个人在，即使再紧急也能完成高质量的工作。

我们取得的成果：

• 尽管此次的工作万分紧迫，但我们仍然建立了完整的测试覆盖范围。 因此没有留下开发债务。

• 所有Bug都在审查代码的时候被捕获。

• 队友之间的友情和彼此的信任空前高涨！

我加入这个团队快一年了，到了这种时候，以前由于对代码太过于熟悉，我会不禁想：我是不是应该换个团队？然而，现在我需要考虑不同的事情：我的工作有挑战性吗？发展蓝图上有些什么？我的职业发展计划是什么？我不打算在此做深入讨论。我想谈一谈为什么我会如此喜欢这个团队，以至于这一次我根本不想换团队。

在经过仔细的思量之后，我意识到我喜欢这个团队不是因为团队中都有谁或者我们从事哪些工作，而是因为我喜欢我们的工作方式：

• 整个团队都了解我们的大多数代码，因此我们可以共同探讨设计和体系结构。因此，我们萌生了各种各样的想法，并产生了强壮的技术设计（幸运的是，我们恰巧拥有一支非常多样化的团队）。

• 当某人不在场时，其他人可以临时顶上。

• 我们经常进行重构和实验，而且我们感觉这样做很安全。

怎样才能打造了一支这样的团队？这要完全归功于我们团队建立的PR审查规则以及指南。

当初这个团队建立的时候，我们只有3位开发人员，一位产品所有者和半个经理（另一半忙于管理一个更大的团队）。我们觉得麻雀虽小但五脏应俱全。因此，我们提出了PR审查规则：无论我们其中一位编写了怎样的代码，其他两位都要冷静。接下来，由于我们不断审查彼此的PR，因此这套审查规则又扩展成了一部指南。

下面，我来介绍一些我们的规则和指南，希望能对某些读者有所帮助，也希望能对大家所有启发。

规则1

每个PR审查必须由2个以上的开发人员批准。经理批准不算在内。

首先请注意，刚开始的时候我们只有3个人，所以这种方式很完美。三个开发人员能够百分百了解所有的代码。对于较大的团队来说，情况可能会有所不同。就像哈利波特里面邓布利多的魂器一样，即便你死了，至少还有2-3个人拥有魂器（伏地魔制造了六个魂器，第七个在哈利波特体内）。现在我们团队有5位开发人员，其中两位是老人，一位新来的，还有两位来了一段时间了。我们仍在尝试建立一种PR模型，保证每个人都能够审查每个人的PR，可能最好的组合是：1位老人+1位其他开发人员。

“经理批准不算在内”？或许你会想：“什么？我是一位经理，也是一位优秀的程序员，我的审查非常重要，你怎么能说不算数呢？”经理审查确实有帮助。我们的经理很优秀，他们都曾是优秀的程序员，他们有时间进行审查，而且他们常常能够提供一些技巧或想法来帮助我们。

但是，你必须考虑经理能够在编程上花多少时间，会超过50%吗？如果会的话，那么他们也算开发人员，那我们应该将经理的批准算在内。然而，在我们公司，开发经理写代码的时候少之又少。归根结底，与代码共同进退的还是开发人员。因此，开发人员拥有最终决定权。我真的不喜欢看到经理卷入开发人员的代码审核中：经理不断地批准某位开发人员的PR，而团队中还有3位其他开发人员都没有机会看一眼PR中的代码。

规则二

每个PR必须拥有良好的描述。审查者能够通过阅读描述理解代码的含义。即使这段代码对应Jira上的票据或有功能需求描述，也不可以省略这一步。

在我们团队中，没有描述的PR永远无法通过审查。如果幸运的话，他们没有写描述，但Jira票据上有详尽的描述。但不幸的话，Jira票据上可能什么都没写。

如果没有描述，那么在审查PR时开发人员需要做两件事：

1. 通过阅读PR中的代码变更来理解代码的作用。

2. 审查者搞清楚代码是否实现了应有的功能。但如何才能搞清楚呢？根据审查者最普遍的理解以及当前的地缘政治气候吗？

如果没有明确说明代码的含义，则无从检查代码的正确性。因为你根本不知道什么是正确的。你只能根据自己的假设做判断。

规则三

PR必须拥有充分的单元测试和集成测试覆盖范围。

在理想情况下，审查者可以轻松地查看测试涵盖的用例列表。

如果测试范围已经存在，或者由于某种原因测试范围不完整（部分工作分解到了另一个票据中，或者有依赖项等），则PR描述中应提及这一点。

良好的代码审查既困难又耗时。这是因为在你了解代码的功能后，就必须停止审查并速度整理你想要的测试范围。你可以建立自己的测试用例列表，并确定测试的种类：单元测试/集成测试/端到端测试。

你还必须考虑是否会对生产造成影响，以及是否需要在部署前对实时数据进行任何测试。

在掌握这些情况后，你可以重新回到PR，检查代码是否涵盖了你考虑到的所有情况。理想的情况是好坏参半：有些情况你想到了，而代码没有；而有些代码做到了，你却没想到；你们结合到一起就可以更好地完成工作。相对于需要阅读100或1000行测试才能搞清楚实际需要测试的用例，通读PR涵盖的测试用例列表就简单多了。

请注意，如果代码遵循了规则2，则只需阅读描述即可理解代码应执行的操作。因此，可以先检查测试覆盖率，再审查实际的代码！

我个人喜欢在PR描述中记录所有内容，包括测试、实时数据测试、实时客户端含义/产品配置更改等等。当然你可以将Jira票据链接进去。

规则四

如果PR是一个bug修复，则必须包含这样一个测试：如果还原代码变更，则这个测试会失败。

我认为这种做法很显然，但与大多数我们看来显而易见的事情一样，这种做法也并非很明显。

假设你错误地将==输成了!=。当时已是晚上10点，其实你不应该在如此疲劳的时候编写代码，但是那天晚上你感觉要加倍努力。于是，你打开了某个bug修复PR，并将!=改成了==，然后这个PR被某个勤劳的好心人批准，并推送到了主分支。

三天后，好友D终于准备好了自己的PR，但意外发生了：他觉得这是一个小功能，因此没有分解成更小的票据，然而实际情况并非如此，结果他的PR中包含了2000行的代码，他请求团队成员审查这个PR。

D进行了代码合并，但在遇到你的修改：将!=改成==时发生了冲突。D很累，他没有注意到具体发生了什么，于是，被修改掉的!=就又回来了！

由于没有测试捕捉这个bug，所以它又回来了。

这是一个人为的错误，但如果你细心留意，就会发生这种事情层出不穷。仔细看看某些存在了3年以上的bug，你就会发现这些都是未被测试捕捉到而再次出现的bug。

总的来说，真正的规则其实就是以上三条，第四条更像是常识。

我认为，这些规则引导我们团队产出了大量的高质量代码。我对此感到非常高兴，而且这绝不是某一个人的功劳。

原文：https://medium.com/inside-league/how-one-code-review-rule-turned-my-team-into-a-dream-team-fdb172799d11

更多精彩推荐
☞去世这天是她的生日，全球首位女性图灵奖得主 Frances Allen 的传奇人生
☞华为云 GaussDB 数据库，会是新的国产之光吗？
☞小米十年，雷军的一往无前
☞用Bi-GRU语义解析，实现中文人物关系分析
☞CPU：别再拿我当搬砖工！
☞DeFi升空助推器：收益耕作者「Yield Farming」
点分享点点赞点在看