怎么进行Code review?

需求

• 功能是什么？
• 功能之外的需求
  • 可用性/易用性、
  • 可测试、
  • 目标性能、
  • 安全性、
  • 扩展性、
  • 可用性等
  • 监控、打点

设计

1. 方案设计是否合理

1. 表定义：是否需要分库/分表？根据业务预期增长情况考虑、2～3年
2. 缓存：Redis、本地缓存、
3. Redis数据结构设计是否合理：zset、hash、string、set等

- 大key问题？
- 热key问题？

3. 数据结构/模型定义
4. 相关分层结构？

- 同样的代码逻辑是否集中在同一个服务/模块中，而不是零散在多个微服务、定时任务等多个地方？
- 读写DB、修改缓存是否集中的同一个服务和模块中

5. 是否需要消息队列/异步等？

2. idl设计

1. 接口设计是否合理，接口参数是否合理，
   • 1. 命名规范容易理解、
   • 2. 必要的注释、
   • 3. 是否容易使用，不容易误用、
   • 4. 是否方便测试
2. 效率与性能：
3. 是否具有可扩展？
4. 怎么做到可扩展，一个建议是面向抽象/接口编程，而不是面向具体编程？比如我们设计接口的时候，不要固定具体类型、参数、字段；依赖外部组件的时候，依赖抽象接口，而不是依赖具体对象；
   比如下面的例子：

GetDenounceRecordsByInfoAndCtimeResponse GetDenounceRecordsByInfoAndCtime(
1
:GetDenounceRecordsByInfoAndCtimeRequest req)

struct GetDenounceRecordsByInfoAndCtimeRequest{
    1: DenounceType denounce_type,
    2: string info,
    3: i64 ctime,

    255: optional base.Base Base,
}

在我看来，上面的接口定义，就很死，不可扩展，如果后面想基于另外一个参数查询这个，就不好做了。需要定义另外一个函数。

上面这个如果是我会定义成这样：

GetDenounceRecordsResponse GetDenounceRecords(1:GetDenounceRecordsRequest req)

然后在request里面定义一些optional的条件字段，比如Info、Ctime、User、Mtime等。

性能和稳定性

• 支持的qps是多少？
• 响应速度：在qps达到一定量级时候，pct99响应时长是多少？
• 索引
• 缓存是否会存在大key问题？
• 缓存是否会存在热key问题？

安全性

所有ugc内容，都需要检查输入数据是否安全。

• 是否有sql注入
• 是否有xss注入？
  • https://owasp.org/www-community/xss-filter-evasion-cheatsheet
• csrf
• 是否需要过词表、模型、审查？

编码

• uber的go编码规范:
  • 中文： https://www.cnblogs.com/legendtkl/p/uber-go-style-guide.html
  • 英文： https://github.com/uber-go/guide/blob/master/style.md
• 是否存在内存/goroutine泄漏？锁竞争？
• sql语句是否合规？是否使用上索引？
• 空指针？提示语不合法？
• 代码是否具有可测试性？
• 是否有魔数常量？
• 代码是否适合修改、变更？控制开关等
• 日志格式、等级是否规范等？

监控打点

• 是否有监控、打点？
• 数据统计&报表、

参考文献

• Code Review Checklist： https://blog.csdn.net/lyjy1216102/article/details/89046655
• https://github.com/uber-go/guide/blob/master/style.md