wd and cc

— Happy every day

Doing Code Reviews

Posted at — Oct 29, 2019

集中看了一波 code review 的帖子,记录一些笔记

How Do Code Reviews Work at Microsoft?

微软这个倒是没什么亮点,不过最后有一堆别等链接值得一看。

  • code review 包括测试结果

  • code review 包括用户界面,可以通过截图什么的提交,这样方便 review

  • 包括静态检查结果

Code Reviews at Google are lightweight and fast

Google 的方式

  • 提交前运行静态检查工具 Tricorder 检查

  • 所有提交都需要 review

  • 代码是全部组员的,所以所有人都要为代码质量负责

  • 代码可读性的 reviewer 必须要取得一个认证之后才能参与 review

  • review 注重的是 owenership 和 readability,不看 reviewer 的职位年龄,如果总是需要年长的来 approve code,那会有瓶颈

  • 取得认证有难度,但是比起职级要求来说简单多了

  • 代码只有通过了经过认证的 readability 专家和对代码有 ownership 的人的审批才可以合并

  • 想要取得认证,需要像 readability 高级专家组提交代码,他们会仔细检查代码质量,任何微小的错误和潜在的问题都不允许,比如锁进,多余的空格等。

  • Google 的 style guide

  • code review 很轻快,小的1小时内,大的修改 5 小时内,具体怎么做到的,似乎有人写一篇论文。。 https://sback.it/publications/icse2018seip.pdf

  • 大部分时候只有一个 reviewer,人多了会变慢,人少了又可能会损失严谨,损失 review 质量

  • 提交小改动,90% 代码修改少于 10 个文件,大量的代码只修改了 24 行左右,75% 的 reviewer 只有一个人

  • 小改动可以更快速的 review,也可以让 review 反馈更有效,有效的反馈也会让 review 变少,形成正反馈。所以多提交,少改动

  • code review 也会导致每次修改都应该是有价值的

  • 80% 的 review 会需要开发人员重新修改代码

  • code review 不只是发现 bug,也是为了让开发人员写能让别的开发人员看懂的代码,

    • 学习教育,如何写正确的代码

    • 保持代码质量,例如必须的测试用例,代码格式等

    • 保证代码安全共有,避免一个人专断

    • 避免 bug

    • 跟踪代码变更,清楚代码修改的过程

How to give great code review feedback

Code review 的目的是啥

  • Functional Defects: 功能缺陷

  • Problems with the logic: 逻辑存在问题

  • Missing Validation (e.g., edge cases): 缺乏必要的校验,边界条件没处理

  • Usage of API: API 使用有问题

  • Design Patterns: 设计模式不对

  • Architectural Issues: 架构问题

  • Testability: 可测试性

  • Readability: 可读性

  • Security: 安全

  • Naming conventions: 命名不符合约定

  • Team Coding Style: 代码风格不一致

  • Documentation: 缺少文档

  • Use of best practices: 没有使用最佳实践

  • Language-specific issues: 编程语言层面的问题

  • Use of deprecated methods: 使用了不推荐的函数方法

  • Performance (e.g., complexity of the solution): 性能问题,例如解决方案的复杂层度

  • Alternative solutions…: 有其他更好的解决方案

功能缺陷,缺少必要的校验,最佳实践,代码风格,API 使用和设计模式是相对比较有用的几个点。

不要在 review 的 comment 里面提交和讨论一些和本次代码合并无关的内容,我们需要让 review 专注并快速:

  • 别的实现方案。虽然可能另外的方案看着更好,但是关于这个讨论对本次的代码提交没什么用。

  • 技术债讨论和重构。关于历史的技术债和可能的重构,这些应该在另外的线索里面讨论。

  • 将来工作的计划。不要在本次 code review 里面讨论不会放在目前的开发周期里面的未来规划。放在其他地方做这个事情。

  • 为了明白代码实现的问题。这些对合并代码没意义,不要放在这里问。

好的 code review 反馈是那些对代码本身的反馈:本次代码修改是否有问题,是否高质量。其他的和这个目标无关的,可以另外找地方讨论,不要在 review 里面讨论,目标是让本次提交尽快合并。

Proven Code Review Best Practices from Microsoft

Code review 有两方人参与,author 和 reviewer

对于 author:

  • 仔细检查自己要提交的内容,自己发现问题比等别人发现好。

  • 提交小改动。如果不同的需求在一次提交里面,review 会变得比较痛苦。

  • 增加新功能,修复另外一个功能里面的 bug,重构代码,这些都应该在不同的提交里面。应该让每次的提交都有一个清晰的,单一的,简单的目标。

  • 描述清楚修改的目的和动机。

  • 提交前运行测试。

  • 使用一些自动检查的工具,比如格式检查,语法检查,静态检查工具等。

  • 跳过一些简单的修改的 review,比如格式修改,局部变量改名什么的。

  • 不要选择太多的人参与 review。一方面人多了会让每个参与的都觉得自己的 review 没那么重要,另一方面会降低整个 team 的工作效率。

  • 让熟悉这块代码的人参与 review 会有好处。

  • 让新手参与可以让他们学习学习。

  • 通知可能会从这次 review 受益的人。但是也不要通知太多人。

  • 提前让其他参与 review 的人知道需要他们 review。

  • 虚心接受建议。

  • 尊重其他 reviewer。

对于 reviewer:

  • 提出靠谱的,有建设性的意见建议。

  • 有需要的话当面或者通过 im 单独聊聊。

  • 单独交流的内容记得记录,以免其他人不清楚。

  • 解释清楚拒绝的原因。

code review best prectices

  • 把 code review 作为一个每天的例行工作,比如每天上午的 11-12 点。

  • 尽量减少任务切换,这个是生产力的大敌。使用比如固定的时间 review 会比较好,可以一天几次这样。

  • 如果最后参与 review 的人总是没什么建议可以提交,那应该需要减少参与 review 的人了。

  • review 频率高一点可以保证 review 质量。

  • 注意核心问题,不要光注意一些小问题,例如拼写错误什么的。

  • 使用一个用来 review 的 checklist

ref link:

comments powered by Disqus