代码审查

代码审查的整体目标是作为团队其他人的安全网，帮助他们编写更好的代码，而不是评判他们或他们的代码。如有疑问，请假设他们有良好的意图，并且要友好。

目标

• 捕捉错误
• 捕捉方法的非明显后果 - 这个PR是否会使得将来的代码更难安全或更易出错。
• 对于没有经过讨论就编码的情况，代码审查作为一种理智检查，确保采取正确的方法。
• 指出PR对Metabase中PR未触及的部分的影响
• 指出使用良好方法或风格的地点。代码审查不是仇恨节。除非PR完全糟糕，否则应该提出同样数量的优点和缺点。

进行代码审查的心态

作为审阅者，你的主要目标是作为安全网，防止糟糕的代码被合并。"糟糕"的定义非常主观，取决于上下文，并且会随着产品的时间和成熟度而变化。

当你发现明显的错误时，花时间去注意你认为它们是错误的原因。

如果你看到你不认同的方法，请发表意见。然而，也要花时间去理解作者做出某个选择的原因。你应该假设作者基于当时所知道的情况做出了正确的决定。你可能有一套不同的知识，看到不同的结果。深入挖掘这些。他们可能看到了你没有看到的事情，反之亦然。

寻找你可以借鉴的技巧、技术或惯用语。你的队友是聪明的人。他们可能有一些你可以学习的技巧。确保让他们知道。

接受代码审查的心态

审阅者正在为你做一件好事。他们在这里是为了帮助你做得最好。最好的最好有教练、编辑和导师。你的代码审查者应该以同样的方式帮助你。在他们更有经验的情况下，这可以是一对一的辅导。在他们更有经验的情况下，他们有一双全新的眼睛，可能会让你质疑根深蒂固的假设。

当一个审阅者不同意你采取的方法时，努力理解原因。他们可能知道一些事情或看到了你没有看到的后果。虽然他们可能没有像你那样深入思考PR的具体主题，但他们同样可能正在考虑PR对你可能没有注意到的领域的影响。

如果有人在你提交的PR上打上一个强烈的:-1:，请特别耐心。深入挖掘他们认为PR存在缺陷的原因。以改进PR为目的进行对话，而不是为了捍卫自己的方法。辩论能力不能给你加分，但高质量的代码和产品可以，不管灵感和想法从何而来。

流程

• 每个复杂度较高的PR至少需要团队中的另一名工程师(:salsakran)确认:+1:后才能合并
• 将你认为应该审查你的PR的人添加到PR的分配者中。审查者审查后可以自行移除，或者决定自己不适合担任审查者
• 影响其他工程师工作的代码应由这些工程师审查
• :+1:是默认的“我对此没有问题”
• :+0:（我创造的）表示“我不太喜欢这个，但其他人说“+1”意味着它可以合并
• :−1:是一个硬性否决。这应该在不寻常的PR中使用，仅用于缺少测试、公然违反风格指南或破坏代码库其他部分依赖的假设的情况。
• 如果你没有讨论设计或与其他可能受到影响的其他工程师讨论影响就切断了主要分支，你应该预料到得到:-1:，并且不应该纠结于重做有争议的部分。
• 任何得到:-1:的PR都不能合并，直到问题解决。
• PR的所有者和投:-1:的人应解决方法上的差异。
• 如果出现僵局，@salsakran将投决定性的一票。僵局应该是罕见的。

注意，这些:+1:、:+0:和:-1:应在评论中明确表示，而不是在github上PR的主要描述中的反应。从:-1:到:+1:的变化也应明确在评论中表示。

时间安排

• 高优先级问题的PR应尽快进行代码审查。
• 里程碑中的问题的PR可以等待几天。
• 如果PR上没有:+1:，则PR创建者有责任跟进其他人并获取他们的代码审查。重申，PR需要:+1:才能合并，如果没有审查，则PR的创建者有责任找到审查者。
• 如果:-1:没有明确的解决方案，PR创建者和:-1:的投票者应计划在接下来的一天或两天内花一小时讨论问题，并计划如何解决它。
• 如果一周内PR上的:-1:没有进展，@salsakran将介入。

如何提高代码审查的质量

有关代码审查研究摘要，请参阅现实世界中的代码审查是如何工作（以及如何不工作）。

PR作者可以做什么以获得更好的审查

• 通过在代码的重要部分添加评论来指导审查者。
• 如果你需要某人的专业知识/意见，请标记那个人。
• 通过使用笔记和警告来增强PR描述——如果你想让某些信息突出，这些工具非常有效。

PR审查者可以做什么以提供更好的审查

• 请密切关注测试文件。注意我们倾向于对测试文件赋予较低的重要性，并下意识地去彻底审查它们。
• 从对更改最重要的文件开始，而不是从PR中按字母顺序排列的第一个文件。
• 如果您觉得自己缺乏背景知识，请向作者请求更多细节或更好的描述。
• 除非更改是微不足道的，否则请检查该分支，并在本地使用包含更改的Metabase进行测试。确保一切按预期工作。