一条代码审查规则如何让我的团队变成梦之队？

考虑以下场景。

产品负责人大橘向团队中的三个开发人员--三猫, 四狗, 五谷--汇报了他们刚刚交付的报告所需要的紧急修改。第二天上午10点有一个客户会议，修改必须上线，这样大橘才能演示。现在是下午6点。五谷正在通勤回家，四狗正在去别墅的路上（她请了假）。三猫却恰好在他的笔记本电脑旁。

下面是接下来发生的事情。

• 三猫看到大橘的消息，并让大橘知道， 没问题，他可以进行更改。然而他从晚上9点开始和明天整个上午都不在，因为他有家庭事务。因此，他可能无法在早上现场推送更改或支持。
• 四狗，作为后座的乘客，在Slack上看到了这，并说 "没问题，如果有什么需要完成的，我早上会在那里"。
• 三猫开始写一些代码，并打开了一个pull request（PR）。
• 晚上8点，五谷回到家，看了看Slack，让团队知道她现在可以帮忙了。她看了看三猫的PR，发现没有测试，因为这是一个急件，于是决定添加测试。由于需求在Slack上，而且他们可以在彼此的fork上进行协作，这不是一个问题。
• 三猫晚上9点下班。
• 晚上10点，五谷完成了测试，这时四狗已经上线，可以进行审查。四狗抓到了一个bug（五谷在脑海中反转了一些需求，因为她很困），五谷调整了代码后，他们推送上线。
• 到了早上，四狗又做了一些修改，因为大橘在最后一刻提出了一些非常额外的要求，并把它们推送到了网上。三猫从他的手机上批准了第二个PR（他很担心，正在检查）。客户会议取得了巨大的成功。

把性别反过来，这基本上是我团队一个周四深夜的情况。我觉得非常自豪。不过不同寻常的是，这并不是我以前24岁时，为一家创业公司熬夜时那种愚蠢的自豪感。看看我，我的工作很重要 如果我不连续工作24小时，世界就会灭亡

这次我的自尊心被不同的事情所激发。

• 我的团队中的三个开发人员都同样熟悉代码 并同样具备修改和审查工作的能力。
• 整个团队奋起直追，没有留下任何一个人加班到深夜。
• 因为我们有三个人，所以即使在匆忙的情况下也能完成高质量的工作。

结果是

• 尽管这是一项紧急的工作，但测试工作已经全面展开。因此没有开发债务。
• 所有的bug都被审查发现了。
• 团队的友谊和彼此的信心达到了前所未有的高度。
• • *

我在同一个团队已经快一年了，这个时候你对代码已经有点太熟悉了，所以你开始有了想法：s_我要不要换个团队？_现在，要考虑不同的原因：你是否受到挑战？路线图上有什么？你的职业规划是什么？我在这里就不说这些了。我想说的是，为什么我特别喜欢在这个团队工作，以至于我不想换。

经过思考，我意识到我喜欢在我的团队工作，不是因为_谁_在团队里，也不是因为_团队的工作内容。我喜欢的是我们工作的方式。

• 整个团队知道我们所拥有的大部分代码库，所以我们一起讨论/设计和架构。这导致了非常好的各种想法和强大的技术设计（我们恰好有一个相当多样化的团队，对我们来说是幸运的）。
• 当我们中的一些人不在时，我们可以在紧急情况下交换。
• 我们经常进行重构和实验--而且这样做很安全。

我们是怎么做到的？**我们为团队制定的Pull Request审查规则和准则。

团队成立的时候，我们只有3个开发人员，一个产品负责人和半个经理（另一半经理忙于管理一个更大的团队）。我们决定，既然我们人这么少，不如把我们当成一个团队来运作。于是我们想出了一个公关审查规则。无论我们中的一个人写什么，另外两个人都要接受 然后，随着我们不断地审查对方的PR，我们把规则集扩展为一些准则。

我在这里写下这些规则和准则，因为我意识到也许有些人会发现它们对他们的团队有用。也可能这些会给你带来一些思考，你会有完全不同的想法。

规则1

**每次PR审查必须至少有2个同团队开发人员的批准。经理批准不算。

首先要注意的是，由于我们一开始是3人团队，这是最理想的。所有3个开发人员都是100%的知情者。对于更大的团队来说，这可能是不同的。你的目标是邓布利多--Horcruxes的情况，如果你死了，至少有2到3个人知道Horcruxes。我们的团队现在有5个开发人员，其中有2个老家伙，1个新来的，还有2个中坚力量。我们还是想坚持每个人都审核每个人的PR的模式，但如果改成1个老前辈+1个其他开发者的最低限度，可能会比较合理。

"经理审核不算数"--也许这让你觉得 "WHAT，我是经理，而且是个了不起的编码员，我的审核完全算数，你怎么敢。" 经理审核确实_有帮助。我们有了不起的经理，他们恰好是很棒的编码者，当他们有时间审查时，他们经常会有很好的提示或想法来帮助我们。

但你必须考虑你的经理花了多少时间在编码上。是否超过50%？那么他们也是一个开发人员，所以好吧，让我们来计算他们。然而，在我们公司，开发经理偶尔会做非常多的编码工作。到最后，是开发人员要和正在编写的代码一起生活。因此开发人员有最终的发言权。我非常不喜欢看到经理和开发人员无限的审批周期，一个经理一直在审批一个开发人员的PR，即使团队中还有3个开发人员还没来得及看。

规则2

**每个PR必须有一个好的描述。通过阅读描述，评审员应该能够理解代码的目的。即使有Jira ticket或需求页面，也必须如此。

没有描述的PR - 在我的团队中，这种情况永远不会通过。最好的情况是，他们没有写，因为Jira ticket有一个很好的描述。最坏的情况是：Jira ticket没有任何描述。Jira ticket什么都没有。

没有描述的PR的审核者有2个工作。

1. 通过阅读PR中的代码变化，了解代码的作用； 2.
2. 试图根据......审查者对宇宙和当前地缘政治气候的一般看法来决定代码是否做了_它应该做的事情？

如果没有明确解释代码要做什么的声明，审查它的正确性是不可能的。你根本不知道什么是正确的。你是在你认为正确的假设上操作的。

规则3

PR必须有足够的单元测试和集成测试覆盖率。

理想情况下，评审员可以很容易地看到所覆盖的测试案例列表。
如果测试覆盖率已经存在，或者由于某种原因测试覆盖率不完整（工作被分割到另一个票据中，依赖性），PR描述应该提到这一点。

给出好的PR评审是很难的，也是很费时间的。这是因为一旦你了解了代码应该做什么，你就必须停止评审，记下你想要的测试覆盖率_这个。你要制定你自己的测试用例列表，并决定它们是哪种类型：单元测试/集成/端到端。

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

现在你知道了所有这些，你再去回顾PR，检查他们是否涵盖了你想到的所有内容。理想情况下是混合的--你想到了一些他们没有想到的东西，而他们也想到了一些你没有想到的东西，你们一起做得很好。这就是能够轻松地看到PR中覆盖的测试用例列表的地方（而不是必须阅读100行或1000行的测试，并且必须弄清楚哪些用例是真正被测试的）。
需要注意的是，如果遵循了规则2，你只需要阅读描述就可以知道代码应该做什么。所以，你可以在审查任何实际代码之前，就进行审查测试覆盖率的步骤!

我个人喜欢把所有这些--提到的测试、任何实时数据测试、任何实时客户端影响/需要的prod config变化--都放在PR描述上。这样一来，它就在一个地方。可以是一个链接的Jira ticket。只要它在某个地方，并且已经被考虑过了。

规则4

**如果PR是一个bug修复，它必须包含一个测试，如果bug修复被恢复，这个测试将失败。

我以为这是显而易见的，但就像大多数显而易见的事情一样，它不是。

假设有一个排版错误，你用了 != 而不是 ==。当时是晚上 10 点，你本不应该进行编码，但你觉得那天晚上你想多走几步路。于是你开了一个bug修复PR，把 != 改成了 ==。一些善良的人，也在工作，批准了它，然后把它放到了主分支。

三天后，你的好友三猫终于准备好了他那份失控的PR：他以为这是一个小功能，没有把它分解成更小的ticket，但结果并不是这样，现在他有一个2000行的PR，他在求他的团队审核。

三猫从上游合并，代码冲突的地方你把==换成了!=.三猫很累，他没注意到怎么回事，!=又回来了。

因为没有测试来抓它，所以bug又回来了。

这是一个人为的例子，但如果你在较长的时间内注意--这种情况一直在发生。把3年内的bug摆出来，你会发现那些没有被测试捕获的bug又出现了。

---

就这样吧! 三条规则，还有第四条更多的是常识性的项目。

我意识到，这些规则，导致我们团队产生了一些质量相当不错的代码。我对此非常高兴，绝不居功自傲。

同样，这些规则可能你会觉得不对，也可能它们恰好对你有帮助。如果它们引发了任何思想或想法，我很高兴!