注释
开发者有没有写出清晰易懂的注释？所有的注释都是必要的吗？通常注释应该解释清楚为什么这么做 ， 而不是做了什么 。如果代码不清晰 ， 不能清楚地解释自己 ， 那么代码可以写的更简单 。当然也有些例外（比如正则表达式和复杂的算法 ， 如果能够解释清楚他们在做什么 ， 肯定会让阅读代码的人受益良多） ， 但大多数注释应该指代码中没有包含的信息 ， 比如代码背后的决策 。
变更中附带的其他注释也很重要 ， 比如列出一个可以移除的待办事项 ， 或者一个不要做出代码变更的建议 ， 等等 。
注意 ， 注释不同于类、模块或函数文档 ， 文档是为了说明代码片段如何使用和使用时代码的行为 。
风格
在谷歌内部 ， 主流语言甚至部分非主流语言都有编码风格指南 ， 确保变更遵循了这些风格规范 。
如果你想对风格指南中没有提及的风格做出改进 ， 可以在注释前面加上“Nit:” ， 让开发人员知道这是一个你认为可以改进的地方 ， 但不是强制性的 。但请不要只是基于个人偏好来阻止变更的提交 。
开发人员不应该将风格变更与其他变更放在一起 ， 这样很难看出变更里有哪些变化 ， 让合并和回滚变得更加复杂 ， 也会导致更多的其他问题 。如果开发者想要重新格式化整个文件 ， 让他们将重新格式化后的文件作为单独的变更 ， 并将功能变更作为另一个变更 。
文档
如果变更改变了用户构建、测试、交互或者发布代码相关的逻辑 ， 检测是否也更新了相关文档 ， 包括READMEs, g3doc页面 ， 以及任何相关文档 。如果开发者删除或者弃用某些代码 ， 考虑是否也应该删除相关文档 。如果文档有确实 ， 让开发者补充 。
代码细节
查看你整个Code Review中的每一行代码 ， 比如你可以扫到的数据文件 ， 生成的代码或大型数据结构 ， 但不要只扫一遍手写的类 ， 函数或者代码块 ， 然后假设它们都能正常运行 。显然 ， 有些代码需要仔细检查 ， 这完全取决于你 ， 但你至少应该理解所有的代码都在做什么 。
如果你很难看懂代码 ， 导致Code Review的速度慢了下来 ， 你要让开发者知道 ， 并且在Review前澄清原因 。在谷歌 ， 我们有最优秀的工程师 ， 你也是其中之一 。如果你都看不懂 ， 很可能其他人也看不懂 。所以要求开发者理清代码逻辑也是在帮助未来的开发者 。
如果你理解代码 ， 但又觉得没有资格做代码评审 ， 确保有资格的变更评审人员在代码评审时考虑到了安全性、并发性、可访问性、国际化等问题 。
上下文
看改动上下文代码对Code Review很有帮助 ， 因为通常Code Review工具只会显示改动部分上下几行代码 ， 但有时你必须查看整个文件确保这次变更可以正常运行 。例如 ， 你可能看到加了4行代码 ， 但是看到整个文件时才会发现这加的4行代码是在一个50多行的方法里 ， 这个方法现在应该被拆分为多个小的方法了 。
Code Review时考虑到整个系统的上下文也很重要 ， 这次改动提升了系统健康度？或者增加了系统复杂性？少了测试用例？…… 不要通过哪些会损害系统健康的代码 。很多系统变复杂都是因为一点一点的小改动日积月累造成的 ， 所以千万不要放进来任何增加复杂性的改动 。
亮点
如果你看到变更中做的好的地方 ， 告诉开发者他们做的很棒 ， 尤其是当他们用某种很精巧的方式解决你评论中提到的问题时更应不吝赞美 。Code Review过多关注于错误 ， 但你也应该为开发者展现出好的一面点赞 。在指导他人的时候 ， 有时候告诉开发者正确的做法比告诉他们错误的做法更有价值 。
总结
在做Code Review时 ， 你需要注意以下：
- 代码设计良好 。
- 代码功能对用户有用 。
- 任何UI改动应当是深思熟虑过而且看起来不错的 。
- 保证线程安全 。
- 代码没有增加不必要的复杂性 。
- 开发者没有写有些将来需要但现在不知道是否需要的东西 。
推荐阅读
- 科比坠机身亡真相大白 科比坠机身亡现场
- 冲泡正山堂金骏眉最佳水温是多少
- 公司电脑无法上网，是mac地址与另一台电脑的冲突了？
- 盘点国内常用CDN公共库
- 兔毛内胆派克服多少钱 派克服内胆是包胆好吗
- CSRF攻击与防御
- IP地址、子网掩码、默认网关、网络地址、广播地址都是什么意思？
- 接吻一直伸舌头的男人是什么性格 男生搂女生腰能感觉出粗细吗
- cgi、fastcgi、PHP-fpm都是什么
- 蜜獾是世界上最厉害的动物吗 什么动物能打败蜜獾
