如何做codereview？本文分享Shopify的6个非常有用的代码审查实用技巧。代码审查是建立有效团队的重要方面，这已成为共识。关于这个话题，已经有很多文章讨论过，比如这篇论文——《An Empirical Study of the Impact of Modern Code Review Practices on Software Quality》。实际上，许多公司的无数团队都进行了某种形式的代码审查。https://sail.cs.queensu.ca/Downloads/EMSE_AnEmpiricalStudyOfTheImpactOfModernCodeReviewPracticesOnSoftwareQuality.pdf但实际情况是，codereview刚开始的时候，大家的热情高涨，之后codereview就流于形式，要么反馈不及时明确或让人难以执行。从长远来看，这会导致团队错失更快学习、共享知识并最终提高代码质量的机会。在Shopify，我们不仅仅着眼长远，我们还想走得更快。根据我们的经验，良好的代码审查实践对工程师的成长和我们构建的产品的质量有着巨大的影响。1.噩梦般的编码经历相信很多人都熟悉这样的场景：你刚加入一个新的团队，领导很快就给你分配了编码任务。作为一个新人，你特别想表现自己，因为你想展示你的编码技能。所以，这就是你接下来要做的：你疯狂地输入代码三个星期来完成工作；您提交了一个包含大约1,000行新代码的PullRequest以供审查；你得到两条关于代码风格的评论，以及一条关于审阅者说他不理解这些代码的目的的问题；你固定代码风格并回答审阅者的问题，然后审阅者批准你写的代码；你将代码分支合并到Master中，闭着眼睛，握紧拳头，咬着牙等待结果。几分钟后，CI完成。还好师父没有坠机。然而……在接下来的6个月里，您一直担心您的代码何时以及如何崩溃。以上噩梦般的经历你可能都有过，那我们就来说说如何改进这个过程吧！2.实用的代码审查实践在Shopify，我们重视交付速度、学习速度和长期成长。这些价值观虽然有时会发生冲突，但会引导我们不断尝试许多新技术并推动团队变革。在本文中，我整理了一系列在Shopify中使用的实用技巧。通过这些技术，我们可以交付经得起时间考验的有价值的代码。术语说明：我们将拉取请求(PR)定义为在合并到基本分支之前经过代码审查的工作单元。Github和Bitbucket的用户都熟悉这个术语。将PullRequests拆分成更小的代码段很简单，并且可能是改进代码审查工作流程的最有用的技术。它之所以有效有两个主要原因：审阅者在心理上更愿意开始和完成对一小段代码的审阅。较大的PR自然会导致审稿人拖延和拖延审稿，审稿过程中被打断的几率更大。作为审稿人，太长的PR很难深入挖掘。我们需要检查的代码越多，我们就越需要花费更多的精力来理解整个代码块。将PR拆分为更小的代码片段可以让您有更多机会在更短的时间内进行更深入的审查。目前，我们无法设定一个适用于所有编程语言和所有类型工作的通用标准。对于内部数据工程项目，原则上我们希望将PR控制在200-300行代码。如果超过这个阈值，我们通常将它分成更小的块。当然，我们也需要注意不要将PR拆分得太小，因为这意味着reviewer可能需要检查几个PR才能了解整体逻辑。使用DraftPRs你有没有听说过造车和喷漆之间的类比？类比是这样的：一个用户要求你造一辆汽车；6个月后，你造出了一辆漂亮的保时捷；你向用户展示汽车，他们问你是否适合他们5个孩子和冲浪板。显然，这里的问题是目标不明确，团队没有收集到足够的反馈就直接构建解决方案。如果在第一步之后，我们先画出汽车的草图，拿给用户看，他们也会问同样的问题，这样我们才能进一步了解客户的需求。这为我们节省了6个月的工作时间。软件也不例外，我们可能会犯同样的错误，将大量工作放在用户不需要的功能或模块上。在Shopify，WorkInProgress(WIP)PR通常用于获得早期反馈，其目的是验证方向（算法的选择、设计、API等）。尽早进行更改可以避免在细节、润色、文档等方面浪费精力。作为编码员，这意味着对改变工作方向持开放态度。在Shopify，我们信奉的原则是让人们有自己的理解，但不坚持自己的意见。我们希望您能自信地做出有充分理由的决定，同时也乐于学习新的更好的替代方案。在实践中，我们使用Github的DraftPRs，它们清楚地表明这项工作仍在进行中，并且Github不允许您合并DraftPR。其他工具可能有类似的功能，至少你可以在创建普通PR时添加WIP标签，以明确工作仍处于早期阶段。这将帮助您的审阅者专注于适当的领域并提供适当的反馈。https://engineering.shopify.com/blogs/engineering/scaling-mobile-development-by-treating-apps-as-services每个关注点的一个PR除了行数之外，另一个要考虑的维度是您的工作单元试图解决的问题问题的数量。关注点可以是功能、错误修复、依赖项升级、API更改等。您是否在重构时引入了新功能？一次修复了两个错误？同时引入类库升级和新服务？将PR分解为单独的关注点具有以下效果：更多独立的审查单元，这意味着更好的审查质量；受影响的人更少，因此他们可以聚集在更少的专业领域；可以回滚原子回滚、小提交或PR。这是很有价值的，因为如果出现问题，可以更容易地确定错误是在哪里引入的以及要回滚哪些部分。把简单的东西和困难的东西分开。假设有一项新功能需要重构常用的API。您可以更改此API，升级十几个调用站点，并实现此功能。80%的更改不是功能更改，显然可以忽略，而20%是新代码，需要仔细注意测试覆盖率、预期行为、错误处理等，并且可能会经历多次修订。对于每次修订，审稿人都需要遍历所有修订以找到相关部分。通过将其拆分为两个PR，很容易快速完成大部分工作，并优化审查以专注于困难的事情。如果您最终得到包含多个关注点的PR，那么您可以将其分解成单独的块。这样就可以对每个piece进行单独review，每个review的迭代周期可以更快，从而加快本次PR的整体review周期。通常，一些工作可以先快速完成，以避免代码变得不可用而导致合并冲突。将PR分解为单独的关注点上面的PR示例包含三个不同的关注点，我们将对它们进行拆分。如您所见，每个审阅者需要检查的上下文要少得多。最重要的是，一旦审查的任何部分完成，代码作者就可以在等待其他审查反馈的同时开始处理已报告的问题。在最极端的情况下，代码作者会陆续收到review的各个部分的反馈，几乎可以不间断地处理这一系列的PR，而不是在完成初稿后等待几天（已经忙于其他things)，最后再回到head去处理反馈。专注于代码，而不是人专注于代码，而不是人。这种做法讲的是人与人之间的沟通方式和关系。从根本上说，这是一种倡导，我们试图将重点放在如何改进产品上，避免作者将评论视为个人批评。以下是您可以遵循的一些提示：审阅者可以思考：“这是我们自己的代码，我们如何改进它？”给个正面意见！如果看到部分代码写的不错，加个评论表扬一下。这使代码的作者保持在他好的一面，并帮助他在心理上更容易接受改进建议。代码作者不妨这样想，审稿人的出发点一定要好，不要把评论当作个人批评。下表列出了一些有缺陷的评论反馈，以及如何按照上面的建议重写它的建议。归根结底，代码审查为我们提供了相互教学和学习的机会，我们应该开放和欢迎。选择合适的审稿人决定由谁来审稿您的工作通常具有挑战性。以下问题可作为参考：谁拥有您正在构建的功能或组件的上下文？谁精通您正在使用的语言、框架或工具？谁知道这个话题足以有自己的理解？谁在乎你写的代码的结果？谁应该学习这些东西？或者，如果你是一个正在复习“老手”的菜鸟程序员，趁这个机会多多提问，多多学习。不要害怕你的问题很幼稚，强大的团队会抽出时间来分享知识。无论您的团队遵循哪些原则，请记住，作为代码的作者，您有责任寻求并从合适的人员那里获得高质量的代码审查。为审稿人提供关键背景最后但并非最不重要的一点是，您的PR描述很关键。根据您选择的审阅者，不同的人会有不同的背景。代码作者有责任提供关键信息或指向更多上下文的链接，以帮助审阅者提供有价值的反馈。你可以将以下问题放入你的PR模板中：为什么这个PR是必要的？谁将从中受益？会出什么问题？您是否考虑过其他方法？你为什么决定采用这种方法？这对其他系统有何影响？好的代码不仅没有错误，而且非常有用。作为代码作者，请确保您的PR描述将代码链接到团队的目标，最好是链接到积压工作中的功能或错误描述。作为审阅者，您将首先审阅PR描述。如果不完整，则无法判断代码是否适合未定义的目标。最好在查看代码之前将其键入。请记住，有时代码审查的最佳结果是意识到代码永远不需要！3.我们会得到什么？通过采用上述一些技术，您可以极大地影响软件构建过程的速度和质量。但除此之外，还有潜在的文化影响：团队将达成共识。团队会更多地了解你的工作，除你之外的其他团队成员也可以改进这部分代码库。团队将分担责任。如果出现问题，需要修复的不仅仅是一个人的代码，而是整个团队的代码都需要修复。任何团队成员都应该能够休假几天，而不会使公司面临风险或因担心世界末日而不停地查看电子邮件。4.个人如何改进团队的代码审查流程？如果您是团队领导者，请开始尝试这些技巧，找出适合您团队的方法。如果您是个人贡献者，请与您的主管讨论为什么您认为代码审查技术很重要，以及它如何提高效率并帮助团队。在您的下一次一对一或团队会议中，探讨这个问题。
