提升代码审查：最佳实践指南

要点

引言

代码审查同时在多个运营维度产生价值。其主要功能是缺陷检测,但二次效果 — 知识转移、一致性强制和问责制 — 随时间推移积累成结构性改进,这些是个别审查会议看不见地产生的。具体来说,代码审查有助于:

• 提高代码质量: 外部视角识别作者在同一代码库上长时间工作后可能会错过的逻辑错误、潜在错误、安全漏洞和性能问题。结果是更稳定和可靠的软件。
• 传播知识: 当一名开发者审查另一人的代码时,他们同时在学习新方法、模式和项目特定的决策。这是团队内最有效的知识转移机制之一 — 对于新员工培训和分散对复杂子系统的理解尤其有价值。
• 确保一致性: 代码审查强制统一的编码风格、结构模式和架构惯例。这种一致性对长期可维护性至关重要,特别是当团队组成随时间变化时。
• 加强团队合作: 代码审查是一种协作行为,创建了一个开发人员相互支持成长的环境。结果是更紧密和绩效更高的团队。
• 减少技术债务: 定期审查及早识别和解决有问题的决策,在它们嵌入代码库并变得难以消除之前。
• 增强问责制: 知道代码将由同事审查,从一开始就为产生更深思熟虑、可读和结构良好的工作创造了自然的激励。

审查准备

在提交代码进行审查之前的准备减少了审查者的开销,并增加了所花费的审查时间的价值。

• 分成小部分: 避免提交跨越多个文件和函数的大规模更改。更小、更集中的更改更易于审查和理解 — 操作目标是每个拉取请求100-200行更改的代码。当更改较大时,将其分解为可以独立审查的逻辑单元。
• 自我审查: 作者在提交前的审查 — 验证代码编译、测试通过、逻辑合理、格式一致、名称清晰 — 减少了审查者必须提供的机械反馈量,并将审查重点放在实质性问题上。
• 全面描述: 提供清晰和完整的拉取请求描述:更改了什么、为什么更改、解决了什么问题以及更改如何与项目目标相关。识别需要特别关注的方面。任务跟踪器项目的链接是必需的。
• 删除注释和未使用的代码: 拉取请求应该只包含功能代码。注释的片段和未使用的变量增加了模糊审查中更改的噪音。
• 本地测试: 所有自动化测试 — 单元和集成 — 应该在提交之前在本地通过。任何所需的手动测试应该在拉取请求描述中明确描述。

文化和沟通

有效的代码审查取决于所涉及的人际互动的质量,而不仅仅是技术过程。管理审查的文化规范决定了它是作为一种富有成效的实践还是团队摩擦的来源。

• 要建设性,不要批判性: 审查针对代码,而不是作者。面向代码的短语 — "这可以改进"或"如果我们试试这个呢?" — 比面向作者的评估更具生产力。
• 建议解决方案,而不仅仅是问题: 当识别出缺陷时,提出具体改进比仅仅标记问题更有价值。"在这里使用forEach会提高可读性"比"糟糕的循环"更具可操作性。
• 询问,而不是指示: 引导作者找到正确解决方案的问题 — "你在这里考虑过工厂模式吗?" — 通常比直接纠正更有效,特别是在培养初级团队成员方面。
• 要具体: 评论应该清晰且有根据。避免一般性短语。在适用的地方提供示例、文档链接或编码标准的参考。
• 注意语调: 书面沟通使语调难以校准。保持明确的礼貌,在可能模糊时使用直接澄清,可以减少评论被接收为个人批评的风险。
• 回应评论: 代码作者应及时回应审查者的问题和评论 — 解释决策、接受建议或以明确的理由表达不同意见。
• 承认审查者的贡献: 认识到审查者投入的时间和努力可以加强协作动力,使未来的审查更有成效。

审查者的焦点

有效的审查需要一个系统化的方法来评估什么。一致的检查清单可防止重要类别被忽视:

• 功能性: 代码是否完成任务所要求的? 它是否解决了所述的问题?
• 正确性和逻辑: 是否有逻辑错误? 边缘情况是否正确处理? 错误条件(空指针、除以零、网络故障)是否得到处理?
• 安全性: 是否存在潜在漏洞 — SQL注入、XSS、不安全的用户数据处理?
• 性能: 代码是否引入瓶颈? 是否有算法将在预期数据量下产生不可接受的性能?
• 可读性和可维护性: 代码对第一次阅读的人来说是否易懂? 变量、函数和类的名称是否清晰? 在必要的地方是否存在注释? 代码是否遵循团队编码标准?
• 测试: 是否存在新功能的单元测试? 现有的测试是否通过? 是否包含错误修复的回归测试?
• 代码重复: 提交是否引入项目其他地方已经存在的代码?
• 架构和设计: 更改是否与整体项目架构一致? 新代码是否引入反模式?

审查不是按照审查者的偏好重写代码的练习 — 它是针对共享标准的有意义错误和改进的系统检查。

工具和自动化

常规审查方面的自动化 — 风格执行、测试执行、漏洞扫描 — 将审查者的注意力从机械检查转移到实质性的逻辑评估。

1. 支持PR/MR的版本控制系统: GitHub、GitLab和Bitbucket为创建、查看和评论拉取/合并请求提供集中接口,行内评论绑定到特定代码行。

2. CI/CD集成: 由每个拉取请求触发的自动化检查应包括:

• 自动化测试套件: 每次提交时运行的单元、集成和功能测试
• 代码linter和格式化工具: ESLint、Prettier、Black、SwiftLint自动强制执行风格标准,将风格执行从审查者责任中移除
• 静态代码分析: SonarQube、Bandit(Python)、Semgrep在人工审查开始之前浮现潜在错误、漏洞和质量问题
• 依赖项漏洞扫描: 第三方库安全的自动化分析

3. 拉取请求模板: 带有必填字段的标准化PR/MR模板 — 更改描述、任务链接、运行的测试、对审查者的问题 — 确保作者提供审查者进行有效审查所需的上下文。

4. 行内评论: 大多数平台支持链接到特定行的评论,使讨论上下文化,而不要求审查者单独引用行号。

迭代和学习

代码审查不是一个静态过程 — 它应该随着团队和项目的发展而演变。

• 迭代方法: 对于复杂的更改,预期有多轮评论和修订。每次迭代应该产生增量改进,而不是试图在单次过程中达到最终状态。
• 回顾会议: 专注于审查过程的定期回顾会议 — 什么有效,什么造成摩擦,哪些反馈模式反复出现 — 提供系统改进过程所需的数据。
• 学习和指导: 审查是团队内可用的最有效的学习机制之一。初级开发人员从更有经验的审查者那里学习; 经验丰富的开发人员发展指导能力。开发人员提交中相同错误的一致模式可能表明需要结构化培训或结对编程。
• 规则适应: 编码标准和审查标准应随着项目成熟和团队组成变化而演变。服务于小团队的标准可能需要随着代码库规模扩大而修订。
• 及时审查: 延迟的审查阻碍作者的进展并增加集成冲突的可能性。审查周转时间的内部SLA — 通常为24-48小时 — 保持开发流程不间断。
• 保护专注时间: 审查时间应该结构化 — 专门的时间块或分配给多个审查者 — 以防止审查持续打断深度工作。

相关文章:

关于任务可视化和优先级的框架级方法,请阅读用Kanban提升生产力:有效任务管理的技巧。

关于在影响性能之前识别和预防倦怠的方法,请阅读如何避免倦怠:维持福祉的关键策略。

关于使用甘特图进行项目时间线可视化和管理,请阅读什么是甘特图? 可视化和管理项目时间线的指南。

结论

代码审查,以一致的准备标准、建设性的沟通规范、自动化工具和持续改进取向实施,作为知识转移和质量保证系统而不是检查程序运作。其长期回报 — 在降低缺陷率、改善可维护性和分布式团队专业知识方面 — 与上述实践应用的一致性成正比。

推荐阅读

"Code Complete"

编写干净、可维护代码的全面参考,大量涵盖支持有效同行评审的实践。

"The Art of Readable Code"

编写清晰传达意图的代码的实用指南 — 产生实质性而非肤浅反馈的审查的基本先决条件。

"Team Geek"

涵盖软件开发中的人为因素 — 协作、沟通以及决定代码审查等实践在实践中成功或失败的人际动态。