Question

Web 开发大多是团队协作的。假设实现代码的人叫小明，小明在合并代码之前需要考虑如下问题：

1．功能实现是否符合预期，上线后会不会出现非预期的错误？

2．代码有没有影响到本产品或者其他产品现有的其他功能？尤其是那些功能并非小明实现的功能？

3．是否有不合理的设计，代码实现是不是使用了最好的方式，有没有性能问题？

4．实现的代码是否有安全问题？

5．如果小明休假甚至离职了，这部分功能出现了问题，其他人完全不熟悉怎么办？

豆瓣的合并代码有一个不成文的规定：每个 PR 都会需要有作者之外的人来做代码评审，评审者发出至少一个 LGTM（Look Good To Me 的简称）的评论、没有其他未解决的争议、测试通过，这三点要求都达到了才可以让代码合并到主干上。

这样做的好处是：

• 更容易发现一些纰漏。一个人的思考总是会不可避免地出现一些不周全的地方，而这些纰漏在别人眼中也许显而易见。
• 至少保证有另外一个人也了解这个功能的实现。
• 在代码评审中可以从其他同事身上学到很多技巧和经验。
• 促进同事之间的沟通与交流。

项目的代码质量高可以加快开发的速度，因为它会使得迭代、协作和维护更加容易。代码评审是对项目质量的基础保障，提交代码的人当然不希望被吐槽得太狠，所以在提交前会尽量保证代码的质量，并提前为可能收到的一些对需求本身和实现逻辑的提问做好准备。

事实上，要做好代码评审非常难。接下来，笔者分享一些自己的经验和理解。

作为被评审者

当工程师提交了代码，潜台词就是“我认为我写得足够好了，效果还不错”。当其他同事不断地发表评审意见时，工程师容易产生负面情绪，尤其是新进团队的人员：

1．需求这么多，还要花费额外的精力来做或者被做代码评审，有必要吗？

2．某些 Python 规范或者团队规范，比如一行代码不能超过 79 个字符，是不是限制得太苛刻了，不能适当放宽吗？

3．我不熟悉之前某功能，直接拷贝代码，改一下能跑不就行了？

4．这些意见确实有道理，但是时间太紧了，下次改吧。

5．我并不认同 XX 说的，我写的也有道理啊，为什么要改？

6．都是同事，何必吹毛求疵到如此境地，放过我吧！

笔者总结，其实最难的不是自己写代码，而是维护别人写的代码，在复杂的逻辑中找到某一个隐藏得很深的 bug，或者在某个（些）位置添加一些代码以实现新的功能。你需要试着按照最初实现者的思路去理解，这往往是最难的，这个过程中非常让人容易产生挫败感和不良情绪。

如果原作者仍然在职还好，有问题直接去问，但假如他已经离职，你很可能偶然会遇到下面的问题：

1．原作者设计得太复杂，一点小的改进都要大费周章，完全掌控他的代码需要不少时间。

2．代码性能不好。之前因为用户量和访问量太少而相安无事，现在问题突然爆发了，拖慢了整个应用甚至影响到基础设施。

3．想要修改功能时却发现程序里充斥着各种无法理解的逻辑，改完之后莫名其妙的 bug 一个接一个。

遗憾的是，你既可能是那个接手的人，也可能是制造麻烦的人。谁都写过烂代码，笔者偶然翻看自己两年前的代码，也会觉得不忍直视，甚至发生过看了一段代码觉得相当烂，使用 git blame 命令想吐槽原作者，却发现这是自己写的代码这样的尴尬事。

只要不是一个人完成项目就会涉及团队协作，每个人都希望别人写的代码自己能接受，这就需要遵循一些规范，代码评审就是规范代码的过程。其次，代码评审通过评审者从旁观者的角度，小到 SQL 语法，中到程序设计，大到产品方向都给予及时的意见，用更小的成本降低上线后出现问题的概率。

以笔者刚入职豆瓣提的评审意见最多的一个 PR 为例来说明（如图 15.1 所示）。

图 15.1　评审意见最多的一个 PR

发表的评审意见数量为 164。被评审者怎么摆正自己的心态呢？

一开始会被这种严格评审搞得很狼狈，但是当你习惯这种流程并且践行之后，你就是它的受益者。比如之前的那几点，笔者是这么看的：

1．代码评审是代码合并之前不可或缺的一步，代码是团队一起维护的，大家都有责任保证整体质量。

2．每个人都能遵守，我为什么不可以？

3．我经常能看到大量的重复代码，它们的意图和语句很像，长此以往项目会越来越臃肿，每次修改对应的内容都需要把雷同的代码全部修改一遍，如果不能理清这些逻辑，盲目地整体替换，非常有可能造成漏改或者错改而导致上线后出现问题。

4．大量例子证明，当场不解决，大部分就都没有后文了。现在的事情，现在做。

5．世界上有太多的事情不是太阳东升西落这样的客观真理，也没有绝对的压倒性的意见。我们组探索的方案是投票制，少数服从多数。你不一定认同，只需要在团队项目中遵守规则即可。

6．这不是吹毛求疵，而是精益求精，力求完美，你同样可以合理地要求其他人这么做。

作为评审者

作为评审者，笔者也有一些经验和大家分享。

1．尊重他人，就事论事，对事不对人。可以在适当的场景下发一些 emoji 图片、语气词缓解气氛。

2．首先要了解产生此 PR 的一些原因、需求说明等，如果你有强烈的反对意见，多花几分钟审视一下自己的意见再做反馈。

3．询问，不要指点。比如可以说“为什么你要……”，而避免说“不要……”。

4．解释你提出修改意见的原因。

5．评论旨在提高专业技能、产品质量、达成团队共识。很多代码风格或者用法确实是因为个人习惯这么写，大家都可以接受就行了。如果确实觉得需要改善，评论之前可以按以下顺序找到最佳用法的示例再去评论。

• Python 标准库中的用法，最好以 Python 3.5 的风格为标准。
• 知名开源项目中的用法。
• 其他项目中的用法。

人工反馈很可能会由于一时无法控制情绪而用词不当，所以尽量让程序来反馈。为此才引入 mention-bot、给 CI 添加 Flake8 检查、使用 Gandalf 以及其他保证产品代码质量的工具。评审者为了让大家接受自己的意见，必然需要理解需求并找到支持自己观点的各种佐证，这个过程既帮助深入理解了产品，也能提高自己的技术水平。

评审的标准

前面说的都是烂代码，不好的习惯。那么，好的代码是什么样子呢？

Bjarne Stroustrup（C++之父）说：

• 逻辑应该是清晰的，bug 难以隐藏。
• 依赖最少，易于维护。
• 错误处理完全根据一个明确的策略。
• 性能接近最佳，避免代码混乱和无原则的优化。
• 整洁的代码只做一件事。

Michael Feathers（《修改代码的艺术》作者）说：

• 整洁的代码看起来总是像很在乎代码质量的人写的。
• 没有明显的需要改善的地方。
• 代码的作者似乎考虑到了所有的事情。

可以感受到，对好的代码的理解有很多共通的地方：

• 代码简单。你的代码意图明确，其他人才容易与你协作。
• 可读性和可维护性要高。
• 以最合适的方式解决问题。

好的 PR 是什么样子呢？

1．标题要涵盖此 PR 的目标，比如：修复 XXX 问题，优化 XXX 问题，增加 XXX 特性。如果做的修改涉及多个特性，可以使用列表挨个列出来。

2．提供此 PR 起因的说明（比如 Trello 相关链接等），不要假定对方已经熟悉这些事情的前因后果。这样做也是为了在未来别人追溯起来的时候能理解这件事。

3．此 PR 的主体应该包含一个可测试的地址，可以让评审者预览效果。

4．PR 中的每一个 commit 日志都应该可以和代码对应，方便评审。

5．尽量不要发太大的 PR，小步迭代，把整个工程拆分，不断向主干提交。

6．提 PR 之前已经确保在本地或测试环境一切正常。

7．写好的 commit 信息，禁止使用“fix bug”、“commit”这样无意义的 commit 信息污染 commit 树。社区有多种写法规范。推荐参考写出好的 commit message（http://bit.ly/1XYn2Y6 ）这篇文章。另外推荐 Thoughtbot 的 commit 信息模板，使用方式如下：

• 创建~/.gitmessage，输入：

# 50-character subject line
#
# 72-character wrapped longer description. This should answer:
#
# *Why was this change necessary?
# *How does it address the problem?
# *Are there any side effects?
#
# Include a link to the ticket, if any.

• 在 Git 全局配置文件~/.gitconfig 中添加：

[commit]
    template = ~/.gitmessage

以后每次执行 git commit 的时候就会时刻提醒你遵循标准了。

除此之外，被评审者应该非常熟悉 Git 的使用，如 rebase、cherry-pick、reset--soft 等命令。