💡 Key Takeaways
- The Psychology of Code Review: Why Most Reviews Fail Before They Start
- The Reviewer's Checklist: What to Actually Look For
- The Art of the Constructive Comment
- Being Reviewed: How to Make Your PRs Reviewable
我仍然记得那个几乎让我放弃软件工程的代码审查。那是2012年,我在一家金融科技初创公司工作了六个月,刚刚提交了我认为是我们支付处理系统的绝妙重构。高级工程师的审查反馈回来了47条评论——大多数都是“这是错的”的变种,没有解释。我花了三天时间重写一切,结果同一个审阅者只用一个字批准了它:“好”。那次经历没有教会我如何写出更好的代码,却让我学会了如何不进行代码审查。
💡 关键要点
- 代码审查的心理学:为什么大多数审查在开始前就失败
- 审阅者检查清单:实际需要关注的内容
- 建设性评论的艺术
- 被审查:如何使你的PR可审查
快进十二年,现在我是一家C轮SaaS公司的首席工程师,已经审查了8000多个拉取请求,指导了40多位开发人员完成代码审查过程。我见过代码审查拯救公司免于灾难性错误,也见过它彻底摧毁团队士气,导致整个工程部门在六个月内完全更换。两者的区别?不是被审查代码的质量,而是审查过程本身的质量。
代码审查既是软件开发中最强大的工具之一,也是最常被误用的工具之一。根据SmartBear 2023年代码审查状态报告,实施结构化代码审查流程的团队能在生产前捕捉到60-90%的缺陷,但同一项研究显示,73%的开发者报告了负面的代码审查体验,损害了他们的信心或与团队成员的关系。这种矛盾存在的原因在于,大多数团队关注的是审查的内容,而不是审查的方式。
代码审查的心理学:为什么大多数审查在开始前就失败
没有人告诉你的代码审查真相是:它们并不是主要关于代码,而是关于人。每个拉取请求代表了某人数小时的智力努力、创意解决方案和专业身份。当你审查代码时,你不仅仅是在评估逻辑和语法——你是在与另一位人类的工作成果进行互动,这种互动要么会激励他们,要么会打击他们。
在与一位优秀的初级开发人员进行离职面谈后,我通过这个困难的方式学到了这一点,她离开了我们的团队。她告诉我,一个轻视的代码审查评论——“你为什么要这样做?”——让她开始怀疑自己是否适合工程师这个职业。审阅者本意是想真实发问,对她的理由感到好奇,但她将其解读为评判。那时我意识到,异步文本的媒介剥夺了语气、面部表情和肢体语言,只留下了可能以最坏的方式解读的单词。
心理研究也证实了这一点。2021年在《IEEE软件工程学报》上发表的一项研究发现,感知为苛刻或轻视的代码审查评论将合并的时间平均延长了3.2天,并降低了作者未来贡献改进的可能性34%。相反,包含具体赞美和建设性反馈的审查使后续提交的迭代周期快28%,代码质量提高41%。
这并不意味着我们应该对一切都粉饰,或避免指出问题。这意味着我们需要以对面前的人有意图的方式来审查代码。在我写任何审查评论之前,我会问自己三个问题:这是真的吗?这有必要吗?这友好吗?如果我不能对这三个问题都回答是,那我就会重写评论。这一简单的过滤器改变了我们团队的沟通方式,并在过去两年内将我们的平均PR周期时间从4.3天缩短到1.8天。
审阅者检查清单:实际需要关注的内容
当我培训新审阅者时,他们总是会问同样的问题:“我应该关注什么?”答案并不是一份简单的语法规则或风格指南列表——这些应该自动化。你作为人类审阅者的工作是评估机器无法做的事情:设计决策、可维护性和业务逻辑的正确性。
我使用一个四级优先级系统,这帮助我把审阅精力集中在最重要的地方。一级问题是阻塞因素:安全漏洞、数据丢失风险或会导致生产事件的破坏性变化。这些需要立即标记,并附上明确的风险解释。根据我的经验,真正的一级问题出现在不到5%的拉取请求中,但当它们出现时,抓住它们正是我们进行代码审查的全部原因。
二级问题是架构方面的关注点:虽然代码是可运行的,但引入了技术债务,违反了既定模式,或将使未来的更改变得更加困难。这些是审查起来最棘手的问题,因为它们需要理解当前代码库和团队未来的方向。我发现将这些问题表达为问题而不是指令效果更好:“我担心这种方法可能会使下个季度实施功能X变得更加困难——你是否考虑过使用模式Y呢?”这会邀请讨论而不是防御。
三级问题是可读性和可维护性的改进:不清晰的变量名、缺少复杂逻辑的注释,或可以拆分以提高清晰度的函数。这些问题很重要,但并不紧急。我通常限制自己每次审查最多提出三个三级评论,专注于对未来可维护性产生最大影响的领域。超过这个数量,作者就会感到不知所措,停止与反馈互动。
四级问题包括其他所有内容:风格偏好、并不是更好的替代方法,或格式的小批评。这里是我的争议看法:你几乎不应该留下四级评论。如果足够重要需要标准化,就把它加入到你的linter或风格指南中。如果不重要到需要自动化,就不重要到需要拖慢拉取请求的进度。我见过团队花费数小时讨论要使用单引号还是双引号,同时提交代码时夹带实际的逻辑错误。不要做那样的团队。
建设性评论的艺术
有用的代码审查评论和令人沮丧的评论之间的区别往往取决于几句话。比较这两条针对同一段代码的评论:
| 审查方式 | 对代码质量的影响 | 对团队士气的影响 |
|---|---|---|
| 无缘无故挑剔(“这是错的”) | 改善微乎其微;作者没有学到基本原则 | 高度负面;产生恐惧和怨恨 |
| 例行公事的批准(“LGTM”但不阅读) | 没有改善;Bug通过生产 | 短期中性,长期负面因质量下降 |
| 带有推理的解释性反馈 | 高改善;作者学习模式和原则 | 积极的;建立信任和心理安全感 |
| 协作讨论(问题与命令) | 非常高;突显边缘案例和替代方法 | 非常积极;促进知识共享和相互尊重 |
| 自动检查 + 人类洞察 | 最高;自动捕捉机械性问题,人类专注于架构 | 非常积极;减少摩擦,专注于有意义的讨论 |
“这个函数太长,功能太多。” “这个函数同时处理验证和数据转换,这使得单独测试每个问题变得更加困难。考虑将其拆分为validateUserInput()和transformToApiFormat()——这样我们可以测试验证逻辑而无需模拟转换,反之亦然。”
第一条评论在技术上是正确的,但毫无用处。它告诉作者哪里出了问题,但没有说明为什么这很重要,也没有提供解决方案。第二条评论解释了问题,描述了影响,并建议了具体的解决方案。花了我很长时间才明白这一点。