Google 是如何做 Code Review 的?| 原力计划( 九 )


  • 有时 , 您完全信任的自动重构工具已经生成了一个较大的CL , 而审阅者的工作只是检查健全性 , 然后指出他们认为需要修改的地方 。这些CL可能更大 , 尽管会有一些风险(例如合并和测试)仍然适用 。
  • 按文件分类
    拆分CL的另一种方法是通过将文件分组 , 如果这些文件将是独立的更改 , 可以分配各不同的审阅者 。
    比如: 你提交一个修改的CL , 创建一个修改的缓冲区 , 另一个CL的代码修改也可以提交到这里面 。你必须按照顺序提交CL , 但是审阅者可以同时进行审阅. 如果这么做 , 你需要尽可能告诉所有审阅者另一个CL的信息 , 以便他们能得到上线文信息 。
    另一个示例:您发送一个CL进行代码更改 , 而另一个CL则发送使用该代码的配置或实验;如果需要 , 这也更容易回滚 , 因为有时将配置/实验文件推送到生产环境中比更改代码更快 。
    把代码重构分离出来
    通常重构最好不要和功能修改或者bug fix一起提CL 。比如移动或者重命名一个Class , 最好和这个Class的bug fix分开提CL 。这样对于审阅者来说 , 理解每个CL单独引入的更改要容易得多 。
    不过一些小的代码清理工作比如变量重命名可以包含在功能修改或者bug fix的CL种 。这取决于开发者和审阅者的判断 , 当然如果巨大的重构包含在同一个CL中会大大增加审阅的难度 。
    将相关的测试代码保存在同一CL中
    避免将测试代码拆分为单独的CL 。验证代码修改的测试应该进入相同的CL , 即使这样做会增加代码行数 。
    但是根据重构准则 , 独立的测试修改可以单独写入CL 。比如
    • 使用新测试验证先前存在的提交代码 。
    • 重构测试代码(例如 , 引入辅助函数) 。
    • 引入更大的测试框架代码(例如集成测试) 。
    不要破坏结构
    如果您有多个相互依赖的CL , 则需要找到一种方法来确保在提交每个CL之后整个系统都能正常工作 。否则 , 可能破坏代码结构导致后面的开发者浪费大量的时间等你的提交(如果这些CL提交出了问题 , 则更长的时间) 。
    小到不能再小
    有时候 , 您会遇到CL很大的情况 。这很少是真的 。练习编写小型CL的作者几乎总是可以找到一种将功能分解为一系列小的更改的方法 。
    在写大型CL之前 , 思考下是不是能够将重构分离出来 , 这是一个更加清晰的思路 。多和你的队友交流学习下他们对缩小CL的实践和方法 。
    如果所有这些选项都失败了(这种情况很少见) , 那么请事先获得您的审阅者的同意 , 告诉他们一个巨大的CL将要来临 。在这种情况下 , 审查过程会耗费极其长的实践 , 这样请花费更多的时间去写测试代码 , 避免引入bug 。
    如何处理评审者的评论
    当你提交了一个变更做Code Review时 , 很可能你都会收到评审者在变更中的评论 。这里有一些处理这些评论的建议 。
    不要掺杂个人情感
    Code Review的目标是维护代码库和产品的质量 。如果评审者批评了你的代码 , 可以理解为他们在帮你、帮整个代码库、甚至是帮整个公司 , 而不是攻击你或者是质疑你的能力 。
    有时候评审者会情绪低落 , 然后在评论中说出一些令人沮丧的话 , 虽然评审者这样做不对 , 但作为开发者你应当有心理准备 。问问你自己 , “评审者试图与我交流的建设性意见是什么?” 然后照他们说的那些去做 。
    永远不要回应充满怒气的评论 , Code Review工具中违反职业礼仪的情况永远存在 。如果你真的忍无可忍 , 建议先离开电脑也会儿 , 或者干一些其他的事 , 等心情平复下来再回复 。
    通常 , 如果评审者没有以礼貌的方式提供反馈 , 请亲自向他们解释 。如果你无法亲自或者通过视频和他们联系 , 就向他们发一份私人邮件 。友好地告诉他们你不喜欢的事情以及你希望他们做些什么 。如果他们也以非建设性的方式对此私密讨论做出回应 , 或者没有起到预期的作用 , 请酌情上报给您的经理 。如果他们已经以不礼貌的方式回应 , 或者没有取得预期的效果 , 视情况汇报给你的经理 。


    推荐阅读