$ ls ~yifei/notes/

如何提交 Pull Request,如何做 Code Review

Posted on:

Last modified:

如何提交一个 Pull Request

  • 首先,自己 review 一遍代码。连自己这关都过不了,就不要提交了
  • 写一个完整的变更列表。一个合格的 change list 至少应该包含 what 和 why
  • 把改变功能的更改和不改变的(比如调整缩进)区分开
  • 友好面对反驳,虚怀若谷
  • 及时反馈,减少切换上下文消耗,才能更快通过 CR

如何做 Code Review

对于代码的格式、样式和命名以及缺少测试这些问题。在代码审查时讨论这些就有些浪费时间,这样 的检查可以(也应该)被自动化。

哪些要点是只能由人工进行审查而不能依靠工具的呢?

  • 多给例子,如果看着代码有问题,不妨直接把自己的答案填上去
  • 不要说「你」怎么样,而要说「我们」
  • 多夸,问题不大的话就先过。

一些具体的问题:

  • 如何让新代码与全局的架构和风格保持一致?
  • 字段、变量、参数、方法、类的命名是否真实反映它们所代表的事物。
  • 测试是否很好地覆盖了用例的各种情况?它们是否覆盖了正常和异常用例?是否有忽略的情况?
  • 错误信息是否可被理解?
  • 作者是否需要创建公共文档或修改现存的帮助文档?
  • 你对性能的需求是什么,你是否考虑了安全问题?这些是需要覆盖到的重大区域也是至关重要的话题。
  • 接受审查的代码是否涉及之前发布的服务等级协议(SLA)?或这个需求本身有特别的性能需求?
  • 不必要的网络调用:就像数据库一样,远程服务有时也会被过度使用
  • 代码是否用锁来控制共享资源的访问?这是否会导致性能降低或死锁?锁是一个性能开销大户, 在多线程环境中很难理清。考虑使用以下模式:单线程写,其余线程只读,或使用无锁算法。
  • 是否存在内存泄露?
  • 代码是否关闭了连接或数据流?关闭连接或文件、网络数据流很容易会被忘记。
  • 资源池是否配置正确?像资源池是否太小(比如大小设置为 1)或太大(如数百万线程)。
  • 如果超时了,会对系统其他部分造成什么影响?
  • 代码是否使用多线程来运行一个简单的操作?这样是否花费了更多的时间以及复杂度而并没有提升性能?
  • 代码是否使用了正确的适合多线程的数据结构。
  • 代码是否存在竞态条件(race conditions)?多线程环境中代码非常容易造成不明显的竞态条件。
  • 是否存在不正确的缓存失效方式。
  • 条件判断语句或其他逻辑是否可以将最高效的求值语句放在前面来使其他语句短路?
  • 代码是否存在许多字符串格式化?是否有方法可以使之更高效?
  • 日志语句是否使用了字符串格式化?是否先使用条件判断语句校验了日志等级,或使用延迟求值?
  • 检查是否引入了新的依赖。应该使用你所能得到的质量最高的类库。
  • 是否验证了访问权限?
  • 是否存储了 PII 等敏感信息
  • 是否明文存储了密钥
  • 是否有足够的监控?

参考

  1. https://mp.weixin.qq.com/s?__biz=MzIwMzg1ODcwMw==&mid=2247486399&idx=1&sn=219af64c7ecbf9e6c601cefddd0a0eba
  2. https://mtlynch.io/code-review-love/
  3. https://mtlynch.io/human-code-reviews-1/
  4. https://mtlynch.io/human-code-reviews-2/

© 2016-2022 Yifei Kong. Powered by ynotes

All contents are under the CC-BY-NC-SA license, if not otherwise specified.

Opinions expressed here are solely my own and do not express the views or opinions of my employer.