王鹏举代码审核
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阻碍
- 回滚简单
- review时
- 多小
- 一件事情
- 一个特性的一部分
- 包括测试
- 一件事情
大的CL什么时候可以
Python风格
- PEP8
- PEP8 中文版
-
Google Python Style Guide
- 一些PEP
- Python官方文档中的Python常见问题
- typing
来源
其他
pyreverse -ASmy --project Pyreverse $path --output png # 工程结构图
# PyCharm专业版?
cd $path
python -m cProfile -o tmp.pstats tmp.py # 性能分析
gprof2dot -f pstats tmp.pstats | dot -T png -o tmp.png # 性能分析可视化