This is an old version of this page. You can view the most recent version or browse the history.

code review

代码审核

Google的最佳实践介绍

术语

CL: changelist
LGTM: Looks Good to Me

目的

为未来的工程师阅读源码考虑
强迫工程师写出其他工程师能看懂的代码
不止一个人可以熟悉代码，增加知识在公司内部停留的机会
如果找到bug很好，但代码审查的主要目的是提高代码的可理解性

好处

风格、设计的一致性
确保有充足的测试
提高安全性 (不能在没有监督的情况下随意提交代码)

紧急情况

紧急情况下，代码审查需要尽量快
是什么
- 允许一个重要的发布继续而非回滚
- 修复一个严重影响用户生产的错误
- 处理紧迫的法律问题
- 关闭重大的安全漏洞
不是什么
- 希望本周发布而非下周
  - 除非合作伙伴协议有hard deadline
- 开发为一个特性工作了很久，想得到CL
- reviewer在异地
- 在周五前得到CL
- soft deadline
- 回滚CL导致的测试失败或生成中断

reviewer怎么做

目的

保证代码库的整体代码健康状况随着时间的推移不断改进

权衡

developer取得进展
reviewer确保每个CL的质量
reviewer希望代码库保持一致、可维护，对评审的代码有所有权和责任

准则

最重要的准则
- 即使CL不完美，reviewer也应该支持批准，因为它肯定会提高代码库的整体代码健康状况

没有完美的代码，只有更好的
- reviewer不应该要求developer改进CL的每一个小部分
- reviewer权衡
  - reviewer建议的改变的重要性
  - developer取得进展的需要
- 不应该因为代码不完美而推迟数天
如果不重要，reviewer可以用Nit:作为前缀，让developer知道这可以忽略

关注

设计
功能性
复杂性
- 行
- 函数
- 类
- 过度设计
测试
命名
注释
风格
文档
每行
- 假定类、函数、代码块中的内容没有问题
- 确保理解所有代码在做什么
Good Things
- 代码审查通常关注错误，也应该对良好的实践提供鼓励和赞赏
reviewer的礼貌

速度

代码审查缓慢时
- 团队速度变慢
- developer抗议代码审查
- 代码健康影响
如果不是在专注的任务中，在代码审查到来后立即做
代码审查请求的最长时间是一个工作日
一个CL在一个工作日内多轮审查 (如果需要)

编写代码评审注释

摘要
- 善待他人
- 解释理由
- 平衡给出明确的方向与单单指出问题让developer决定
- 鼓励简化或添加注释而非解释复杂性

developer怎么做

好的CL描述

第一行简短命令
空行

错误示例

Fix build
Add patch
Moving code from A to B
Phase 1
Add convenience functions
kill weird URLs

尽量用小的CL

好处
- review时
  - 快
  - 彻底
  - 错误少
- 若被拒绝，成本小
- 容易merge
- 容易设计好
- 减少review阻碍
- 回滚简单
多小
- 一件事情
  - 一个特性的一部分
- 包括测试

大的CL什么时候可以

Python风格

来源

其他

cd xx
pyreverse --project Pyreverse . --ignore test --output png  # 工程结构图
python -m cProfile -o tmp.pstats tmp.py  # 性能分析
gprof2dot -f pstats tmp.pstats | dot -T png -o tmp.png  # 性能分析可视化