Back to Software Engineering

See Also Script for Counting Code LinesGerrit

代码评审

1. Code Review Benefits

很多团队都有这个问题:代码本来开始设计得好好的,一段时间以后,代码就会变得难以理解,难以维护,难以修改。为什么?

最快腐烂的代码,一定有很多人在修改;相反,长期由一个人来维护的代码,就不会那么容易腐烂。因为一个人发生思路冲突的概率要低得多,代码更容易保持一致性;人多思路必然不完全一致,只有靠长期坚持不懈的沟通交流来统一思想。

所以,代码腐烂本质上是沟通问题。改进的办法就是加强沟通。而代码审查是一种很好的社交活动。

1.1. Four eyes catch more bugs

Catch bugs early to save hours of debugging

引入代码评审会缩短测试周期,并降低最终代码缺陷率(Bugs/100Lines)

1.2. Enforce coding standards

Keep overall readability & code quality high

提升代码可读性并提高代码质量--两个人都明白要好于只有一个人(一时)明白;

如果coder编码的时候就很清楚,只有评审通过的代码才能合入主干,那么写出的程序会有所不同:会变得更加整洁,有着较好的注释,结构也组织的不错。如果没有这样的机制,coder知道代码会在最后才会审查。因为不是马上就要检查,所以对他而言并不紧迫,因而不会想着先自检一遍。

1.3. Mentoring of new developers

Learn from mistakes without breaking stuff

帮助新人学习并养成好习惯,对新人最好的监督方法就是代码评审

1.4. Establish trust relationships

Prepare for more delegation

代码评审促进知识共享,增强人力复用。

在很多的开发团队中,每个人都会负责并且专注于一个核心模块。除非别的同事负责的模块出现问题导致自己的代码不能运行,否则他们是不会去关注别人的工作。这样产生的结果是,每一个模块的代码只有一个人比较熟悉。假如事不凑巧,那位程序员正好休假或者离开了公司,那么没有人了解那些代码了。如果有代码审查的环节,那么至少会有两个人熟悉代码——代码的作者和审阅者。审阅者虽然没有作者对代码那么了解——但是他同样熟悉代码的设计和结构。

1.5. Good alternative to pair programming

asynchronous and across locations

另外一种结对编程,不受场地限制

1.6. 提升团队的设计开发能力

代码审查能帮助团队成员从彼此的错误中汲取经验,并成为更好的程序员。通过简单的反馈意见,公司可以提高了其开发员的水平。开发员重视审查,因为他们知道这将帮助他们成长。

当代码审查以小组为单位进行时,整个团队都得以提高。但更好的是,代码质量也得到提升,并易于维护。

1.7. 激发团队凝聚力

人们认为代码审查仅仅是寻找漏洞,但它却能把人团结在一起,它可以提供的远远超过你所预期的。

有很多这样的例子在执行代码审查时发生,但是最好的成功模式是在一个团队成形时就开展审查。你从事某个项目的时间越长,所创建的代码质量就越好。这是因为所有的代码审查过程和管理在项目开始时就开始了。

2. Team Code Review Practices

2.1. 负责人

Team Lead主持本团队的代码审查工作,确保此项工作有效的执行

2.2. 审查时机

pre-merge的具体做法:先创建一个审查区,把修改的代码放进去,然后讨论、等待批准、合并代码(辅助系统会自动合并代码)。

发布给测试的必须是讲过代码审核后的。

2.3. 审查粒度

审查是基于被修改的相关代码,一个单一审查可能会涉及很多次代码提交,审查的批准和拒绝应该是整体的。

建议实现一个新功能时分成三个步骤:

  1. 重构现有的代码,让代码整洁,方便添加新功能;
  2. 加入新功能;
  3. 写单元测试代码。

如果能将多个不同的提交放的一个代码审查中进行,那你就可以简单的将这些修改分组提交。

版本控制系统的最重要的功能就是告诉你代码演变的历史、是如何变成今天这个样子的。我经常会查看昨天代码是什么样的,上周二下午2点代码是什么样的,期间发生了什么变化。很多时候是因为我发现代码以前好用而现在不行,我想知道为什么。而更多时候,我是想知道为什么会对代码做这样的修改。关联的上下文是什么?动机是什么?

2.4. 评审开销

2.5. 审查规范

2.5.1. 编码规范

  1. 编码规范
    • 没有被使用的变量要删除;
    • 针对变量,方法和类要用相同的命名方法;
    • 常量应该被写在独立的常量类中;
    • 每行代码的尾部不要有多余的空格;
    • 对于括号,循环,if语句等等要用统一的格式;
    • 每一个单独的方法不应该超过100行;
    • 一个单独的语句不应该超过编辑器的可视区域,它可以被拆分成几行;
    • 假如类有很多成员变量,并且实例化的时候只需要少数变量传入的话,最好使用静态工厂方法,而不是重载构造函数。
    • 给方法添加适当的访问控制,而不是所有都是 public。
    • Java项目:
      • 检查 String 对象既不是null也不是空的最好方法是 if(“”.equals(str));
      • 针对不同的 Exception 要用不同的 catch 语句,而不是一个 Exception 解决所有问题;
      • 遵守项目中使用的框架的最佳实践建议,例如 Spring,Struts,Hibernate,jQuery;
  2. 注释
    • 每一个类和公共方法都要进行说明;
    • 如果是修复某个 bug,应该添加 bug ID;
    • 走捷径的方法或者复杂的逻辑要有解释;
    • 如果代码会被公开,每个文件头都要标注版权信息;
    • 复杂的脚本应该包含文档
  3. 功能
    • 如果类似的逻辑被使用达到了3次及以上,应该把它写成一个帮助类,然后在多出调用;
    • 鼓励使用 API 而不是重复编写代码解决相同的问题;
    • 要强调代码的单元测试;
    • 任何新加的代码不应该破坏已有的代码;
  4. 安全
    • 任何代码都不能直接执行用户的输入,除非经过转义过了--这个常常包含 JavaScript 的 eval 函数和 SQL 语句;

    • 任何类,变量,还有方法都应该有正确的访问域;
  5. 性能
    • 所有数据库和文件操句柄等资源在不需要的时候都应该被释放;
    • SQL 语句的写法会导致性能千差万别,需要分析线上真实情境进行对应优化(数据规模、操作类型和频度等);
    • 鼓励创建不可变(immutable)的类;

2.5.2. 日志规范

统一日志API:提供一个log输出类,将log输出格式及API统一

级别:输出日志的重要程度,便于控制日志输出的多少

输出内容:

log实例如下:

在什么位置打印日志:

The Problem With Logging 程序打日志需要注意的问题

2.5.3. 单元测试规范

针对以下部分必须有单元测试:

客户端:使用Android Unit

服务器端:使用JUnit

2.5.4. 代码提交规范

3. Peer Code Review Practices(with Gerrit)

3.1. Gerrit Workflow

Gerrit_Workflow.png

3.1.1. One Branch One Feature

Master branch contains only reviewed and approved changes

Each feature branch is based on master branch

A change can really be abandoned because

3.2. Code Review Tools

4. Best Practices

4.1. Code Review Tools

在进行代码评审之前,可以使用一些静态代码检查工具对代码进行初步校验;

此外,有专门的辅助代码审查的工具可用应用到代码审查过程中来,例如Gitlab

支持“批量提交审查”的软件:

4.2. Review Board

Review Board是一个 基于web 的工具,是由 django 和python设计的。 Review board 可以帮助我们追踪待决代码的改动,并可以让Code-Review更为容易和简练。尽管Review board 最初被设计在VMware项目中使用,但现在其足够地通用。当前,其支持这些代码版本管理软件: SVN, CVS, Perforce, Git, Bazaar, 和Mercurial.

➜  reviewboard  virtualenv env
New python executable in env/bin/python
Installing setuptools, pip...done.
➜  reviewboard  source env/bin/activate
(env)➜  reviewboard  pip install reviewboard nose Sphinx
(env)➜  reviewboard  wget http://pysvn.barrys-emacs.org/source_kits/pysvn-1.7.2.tar.gz 
## tar && cd Import
wget http://sourceforge.net/projects/cxx/files/CXX/PyCXX%20V6.1.1/pycxx-6.1.1.tar.gz

4.3. Rietveld

Rietveld 由Guido van Rossum 开发(他是Python的创造者,现在是Google的员工),这个工具是基于Mondrian 工具,作者一开始是为了Google 开发的,并且,它在很多方面和Review board 很像。它也是一个基于Web的应用,并在Google App Engine 上。它使用了目前最流行的Web开发框架 django 并支持 Subversion 。当前,任何一个使用 Google Code 的项目都可以使用 Rietveld 并且使用 python Subversion 服务器。当然,它同样支持其它的Subversion服务器。

4.4. LinkedIn

LinkedIn’s Tips for Highly Effective Code Review

上文是 LinkedIn 介绍的内部评审实践,其代码评审技巧概括如下

5. 敏捷开放的更多实践

5.1. 结对开发

5.2. 测试推动的开发

5.3. 快速迭代

5.4. 每日晨会

6. Reference


CategoryProcess

MainWiki: Code_Review (last edited 2019-08-11 00:01:37 by twotwo)