@42withyou
2015-02-28T09:49:46.000000Z
字数 1321
阅读 567
Justering 代码审查(Code Review)清单
Justering
道德原则
http://blog.jobbole.com/34195/
基本原则
- 在检入到团队的版本控制软件之前,所有的代码都必须进行评审。
- 应该短期高频,一次评审量要低于 200–400 行代码(啊,好像不太适合我们?但多了漏掉问题的概率必然会增加)
- 确定在评审开始之前代码开发者已经注释源代码了。
- 使用检查表,因为它能极大地影响代码开发者和评审者的结果。(一方面,就是这个清单,或简化版本。另一方面,每个开发者应当建立自己常犯错误的清单,作为一个简短的检查列表,一般都是您容易遗忘的事情)
- 提交 CodeReview 之后或之前,应该自己快速过一遍 diff,通常能发现不少细节上的小疏漏。
- It’s OK to say “It’s all good” :)
编码习惯
参考 Justering Tech Team Work Rules 编码习惯一节。
常规项
- 代码能够工作么?它有没有实现预期的功能,逻辑是否正确等。
- 所有的代码是否简单易懂?
- 代码符合你所遵循的编程规范么?这通常包括大括号的位置,变量名和函数名,的长度,缩进,格式和注释。
- 是否存在多余的或是重复的代码?
- 代码是否尽可能的模块化了?
- 全局变量/缓存是否不可用局部变量/缓存替换?
- 是否有被注释掉的代码?有无对于注释的注释?
- 循环是否设置了长度和正确的终止/边界条件?
- 是否有可以被库函数替代的代码?
- 是否有可以删除的日志或调试代码?
功能
- 如果类似的逻辑被使用了多次,应该把它抽象成一个类,然后在多处调用,相信我,你会爱上写类的。
安全
- 任何代码都不能执行用户的输入,除非转义过了。这个常常包含 JavaScript 的 eval 函数和 SQL 语句。
- 所有的数据输入是否都进行了检查(检测正确的类型,长度,格式和范围)并且进行了编码?
- 任何类,变量,方法,还有文件都应该有正确的访问域(private, protected, public)。
- 在哪里使用了第三方工具,返回的错误是否被捕获?
- 输出的值是否进行了检查并且编码?
- 无效的参数值是否能够处理?
- 输出值是否会暴露敏感信息?
文档
- 是否有注释,并且描述了代码的意图?
- 所有的函数都有注释吗?
- 对非常规行为和边界情况处理是否有注释?
- 第三方库的使用和函数是否有文档?
- 数据结构和计量单位是否进行了解释?
- 是否有未完成的代码?如果是的话,是不是应该移除,或者用合适的标记进行标记比如‘TODO’?
- 如果是修复某个 ticket,应该添加 ticket link。
- 走捷径的方法或者复杂的逻辑要有解释。
- 复杂的 HTML,JavaScript,CSS 也应该包含文档。
性能
- 检查是否使用了合适的图片尺寸,CSS sprites 和浏览器缓存等技术。
测试
- 代码是否可以测试?比如,不要添加太多的或是隐藏的依赖关系,不能够初始化对象,测试框架可以使用方法等。
- 是否存在测试,它们是否可以被理解?比如,至少达到你满意的代码覆盖(code coverage)。
- 单元测试是否真正的测试了代码是否可以完成预期的功能?
- 是否检查了参数或数组的值或类型“越界“错误?
- 是否有可以被已经存在的API所替代的测试代码?