[关闭]
@42withyou 2015-02-28T09:49:46.000000Z 字数 1321 阅读 567

Justering 代码审查(Code Review)清单

Justering


道德原则

http://blog.jobbole.com/34195/

基本原则

  1. 在检入到团队的版本控制软件之前,所有的代码都必须进行评审。
  2. 应该短期高频,一次评审量要低于 200–400 行代码(啊,好像不太适合我们?但多了漏掉问题的概率必然会增加)
  3. 确定在评审开始之前代码开发者已经注释源代码了。
  4. 使用检查表,因为它能极大地影响代码开发者和评审者的结果。(一方面,就是这个清单,或简化版本。另一方面,每个开发者应当建立自己常犯错误的清单,作为一个简短的检查列表,一般都是您容易遗忘的事情)
  5. 提交 CodeReview 之后或之前,应该自己快速过一遍 diff,通常能发现不少细节上的小疏漏。
  6. It’s OK to say “It’s all good” :)

编码习惯

参考 Justering Tech Team Work Rules 编码习惯一节。

常规项

  1. 代码能够工作么?它有没有实现预期的功能,逻辑是否正确等。
  2. 所有的代码是否简单易懂
  3. 代码符合你所遵循的编程规范么?这通常包括大括号的位置,变量名和函数名,的长度,缩进,格式和注释。
  4. 是否存在多余的或是重复的代码
  5. 代码是否尽可能的模块化了?
  6. 全局变量/缓存是否不可用局部变量/缓存替换?
  7. 是否有被注释掉的代码?有无对于注释的注释?
  8. 循环是否设置了长度和正确的终止/边界条件
  9. 是否有可以被库函数替代的代码?
  10. 是否有可以删除的日志或调试代码

功能

  1. 如果类似的逻辑被使用了多次,应该把它抽象成一个类,然后在多处调用,相信我,你会爱上写类的。

安全

  1. 任何代码都不能执行用户的输入,除非转义过了。这个常常包含 JavaScript 的 eval 函数和 SQL 语句。
  2. 所有的数据输入是否都进行了检查(检测正确的类型,长度,格式和范围)并且进行了编码?
  3. 任何类,变量,方法,还有文件都应该有正确的访问域(private, protected, public)。
  4. 在哪里使用了第三方工具,返回的错误是否被捕获?
  5. 输出的值是否进行了检查并且编码?
  6. 无效的参数值是否能够处理?
  7. 输出值是否会暴露敏感信息?

文档

  1. 是否有注释,并且描述了代码的意图?
  2. 所有的函数都有注释吗?
  3. 对非常规行为和边界情况处理是否有注释?
  4. 第三方库的使用和函数是否有文档?
  5. 数据结构和计量单位是否进行了解释?
  6. 是否有未完成的代码?如果是的话,是不是应该移除,或者用合适的标记进行标记比如‘TODO’?
  7. 如果是修复某个 ticket,应该添加 ticket link。
  8. 走捷径的方法或者复杂的逻辑要有解释。
  9. 复杂的 HTML,JavaScript,CSS 也应该包含文档。

性能

  1. 检查是否使用了合适的图片尺寸,CSS sprites 和浏览器缓存等技术。

测试

  1. 代码是否可以测试?比如,不要添加太多的或是隐藏的依赖关系,不能够初始化对象,测试框架可以使用方法等。
  2. 是否存在测试,它们是否可以被理解?比如,至少达到你满意的代码覆盖(code coverage)。
  3. 单元测试是否真正的测试了代码是否可以完成预期的功能?
  4. 是否检查了参数或数组的值或类型“越界“错误?
  5. 是否有可以被已经存在的API所替代的测试代码?
添加新批注
在作者公开此批注前,只有你和作者可见。
回复批注