码迷,mamicode.com
首页 > 其他好文 > 详细

Code Review 最佳实践

时间:2019-08-04 00:52:24      阅读:142      评论:0      收藏:0      [点我收藏+]

标签:很多   mit   大量   git   合并   Nging   代码重构   uav   完成   

ref: Code review Best Practices

文章将了以下内容:

  • 3w:why、what、when 进行 code review
  • code review 之前的准备
  • 执行 code review
  • CRs 示例

1.为什么进行CR

  • 代码编写者会规范自己的代码,也会觉得复查别人的代码有一种自豪感;
  • 有益于分享知识
    • 项目中的其他人知道全部的功能
    • 写代码的人使用的算法或技术会被全员学习
    • 加强团队内沟通
  • 保证代码风格统一,减少bug
  • 简洁的代码不止易于复查,也能减少bug

2.复查什么

这个需要在时间和质量之间做选择。有的团队可能仅关注合并和主分支的功能,其他团队可能会对所有代码进行审查。

但是无论范围是什么,需要确定的是,所有人的代码都需要被复查。即使是毫无瑕疵的代码,在审查的过程中,其他人会学到很多编码的好习惯。

3.上线时再进行吗

一般来说复查是在自测、集成测试之后,需要把代码合到主分支之前进行。

4.复查之前的准备

  • 提交者声明代码范围、内容的多少
  • 完整的、自测通过之后的代码
  • 代码重构不应该影响原有功能,相反,增加功能时也不应该重构其他代码。
    • 重构的代码会不重视
    • 大量的重构会不好做遴选,刷新提交等 git 功能

5.提交声明(commit message)

  • 总结
  • 描述做了什么以及怎么做的

6.复查人

  • 1-2个熟悉项目的人,其中一个应该资深
  • 不应该引入其他问题,像代码风格之类的讨论

7.code review

  • 代码功能
    • 提交的代码实现了功能吗
    • 问问题
  • 实现
    • 你会如何实现
    • 可以抽象吗
    • 像对手一样去复查,但是态度要友好
    • 考虑已实现库,减少重复造轮子
    • 实现是否改动了依赖,为什么
  • 易读性
    • 你花多长时间读懂了?
    • 是否符合团队的编码风格
    • 是否有未完成的todo
  • 可维护性
    • 看测试代码
    • 是否向后兼容
    • 是否需要集成测试
    • 提交反馈:是否有逻辑问题?
    • 文档是否更新了
  • 安全性
    • 符合安全规范
  • 注释:简洁、友好

8.反馈

  • 要有反馈,即使是一句done
  • 如果有修改,需要再次review

9.复查示例

9.1 命名不一致

class MyClass {
  private int countTotalPageVisits; //R: name variables consistently
  private int uniqueUsersCount;
}

9.2 方法声明不一致

interface MyInterface {
  /** Returns {@link Optional#empty} if s cannot be extracted. */
  public Optional<String> extractString(String s);
  /** Returns null if {@code s} cannot be rewritten. */
  //R: should harmonize return values: use Optional<> here, too
  public String rewriteString(String s);
}

9.3 重复造轮子

//R: remove and replace by Guava's MapJoiner
String joinAndConcatenate(Map<String, String> map, String keyValueSeparator, String keySeparator);

9.4 bugs

//R: This performs numIterations+1 iterations, is that intentional?
//   If it is, consider changing the numIterations semantics?
for (int i = 0; i <= numIterations; ++i) {
  ...
}

9.5 架构考虑

otherService.call(); //R: I think we should avoid the dependency on OtherService. Can we discuss this in person?

Code Review 最佳实践

标签:很多   mit   大量   git   合并   Nging   代码重构   uav   完成   

原文地址:https://www.cnblogs.com/paxing/p/11296760.html

(0)
(0)
   
举报
评论 一句话评论(0
登录后才能评论!
© 2014 mamicode.com 版权所有  联系我们:gaon5@hotmail.com
迷上了代码!