让评审人爱上你的8个要点

Posted 2021-06-02 阿里云开发者

简介：今天小编作为一个开发者，放下外部的客观因素，仅从一个代码的实现者，一个被评审人的角度去思考如何让评审变得高效而富有意义。换句话说：如何让评审人爱上我（被评审人）。

众所周知，每个有技术追求的团队，都试图想把Code Review做到极致。正所谓——未经审视的代码，不值一提。为什么Code Review如此重要，本篇不再赘述，仅做简单总结：

• 提早发现问题，防患故障于未然
• 快速提高团队新同学的编程能力
  

然而梦想很丰满，现实很骨感，团队的评审活跃度和评审质量往往令人堪忧，缘由种种，上至技术TL、下至代码功能的实现者，甚至产品经理（嗯，一定是业务太重导致没时间做Code Review）都能成为做不好Code Review的罪魁祸首。今天小编作为一个开发者，放下外部的客观因素，仅从一个代码的实现者，一个被评审人的角度去思考如何让评审变得高效而富有意义。换句话说：如何让评审人爱上我（被评审人）。

为什么要让评审人爱上我

在以往的文章讨论中，我们往往更专注于如何提升评审人的水平，如何找出评审代码的问题，或者如何让被评审人能快速解决提出的问题。但我们忽略了评审人的感受，似乎评审人就是一个毫无感情的找BUG机器。当然，不排除一些自动化扫描的工具（比如：Codeup的缺陷检查、漏洞分析等等），它们确实是AI机器人。但除此以外很多时候因为对代码的业务理解、团队的编码风格等问题，还是需要团队的其他成员作为评审者介入其中。对于不走心的敷衍评审我们撇开不谈，评审者参与一次评审还是挺耗费精力和时间的，需要时间去阅读、理解、并指出疑问。因此，我们应该放下防备，坦然对待评审过程，并对评审人心怀感激。

如何让评审人爱上我

既然我们已经下定决心与评审人来一场甜蜜的Code Review之旅，怎样才能让评审人爱上我（的评审）？我从一次评审的全周期来分析，分别为：

• 【评审前】如何做好评审前的准备；
• 【评审中】如何对待评审中的讨论/问题；
• 【评审后】评审后需要做什么。

提交评审前

重点：

• 认真审视自己的代码
• 做好提交（Commit）
• 保证测试基线

认真审视自己的代码

在评审中，最忌讳的一点就是犯一些低级错误，这样不仅会给评审者很差的印象，也会将评审者的时间浪费在一些低级错误之中。所以，我们要在提交代码前自己先认真审视自己的代码。

格式问题

格式问题，常发生在一些初建的团队，大家有不同的开发习惯、编码风格，在单人开发中，并没有什么问题。当这个项目涉及到多人协作时，就会显得很麻烦。比如下面这个评审，其实是没有实际的改动只是编码风格的格式化。但密密麻麻的文件变动，往往会让人心神不宁。
因此在提交前，我们应按照团队的规范，设置好自己的编辑器格式工具，并在提交前检查是否存在格式上问题，避免这种问题在Code Review的时候浪费大家的时间。

拼写错误

拼写错误是属于低级错误了，目前主流的编辑器都支持拼写检查，只要大家认真对待编辑器高亮的warning，就能找出来啦。

代码规范扫描

一些非常基础问题，可以通过自动化工具扫描出来。比如针对java的开发规约检测，代码平台提供的漏洞检测、安全扫描等。在提交以后看到分支上改动的commit都是绿色的pass，心情也会好很多。毕竟咱给评审者提供的代码也是经过一遍初选的复赛选手了。

做好提交（Commit）

说到提交（commit），很多同学都觉得没什么。不就是git commit 么。或者commit的时候，因为Git的强制要求必须包含message，才随便输几个只有自己懂的提交说明，然后git push拉倒。比如下面这种提交说明，我在很多兄弟团队的代码仓库都有见过。

这种提交说明的问题，我就不再赘述了。接过老项目、给老工程填过坑的人都懂。因此在评审阶段，我们就要做好提交。

• 要点一：按功能拆分提交，拒绝大Diff评审
  

在平时的开发中，大家往往都是多个任务并行开发。很多同学又不喜欢来回切换变更的分支，导致很多评审都是多个功能一股脑的提交，因此评审自然也就变得很大。或者在开发一个比较大的需求时，没能将功能进行拆分细化，导致所有的改动都在同一个评审中，甚至在同一个Code Review中。例如：

这个评审，中间混杂了数十个功能的提交，评审者很容易迷失在不同功能的逻辑迷宫中。更有甚者，会提交增删行数高达上万行的评审。对于这种超大评审，我不太相信评审者会认认真真地看完。有研究自媒体分享的统计，现代人的注意力只能聚焦5分钟，如果一个分享超过5分钟无法结束，人们往往会因为想不起来前面的内容而放弃。评审也是如此，长达数万行的改动，都在同一个评审中，即便是在评审页面上提供铆钉已阅读的文件的功能，也很难在短时间内容理解上下文的逻辑。因此评审者需要耗费长达半个小时以上的独立时间来阅读评审的改动，而这对于正常的开发人员是非常崩溃的。因此，我们在完成一个功能的时候，应当合理的按功能拆分，将提交变小（small is beaut iful）。当改动过于庞大应该考虑拆分多个评审进行。

• 要点二：描述清楚本次提交的内容、改动点、改动影响
  

一个评审的开始，从打开页面到评审开始，一般眼睛的顺序是：评审标题->评审描述->改动列表->改动详情

评审的标题和评审的描述往往就是我们提交的内容生成的。而作为评审开始的前两个关键点，评审提交描述是非常重要的。比如下面这种例子：

我相信各位作为评审人，看完这个评审已经想关掉评审了（一些暴躁老哥甚至直接点拒绝）。令人费解的评审标题，空白的评审描述。即便我们拿着Code Review坐在评审者的电脑前、或者钉钉上附上评审解释，这种体验都是非常不好的，因为大家都不愿意注意力被分散。就像我们写代码一样，大家都更愿意在开发过程中聚焦在编辑器上，而不是在不同的屏幕间来回切换（查看需求、被人中途打扰等）。因此我们应该写好描述，尽量让评审开始的时候，大家都能在一块屏幕（一个页面上）完成评审的所有工作。

除此之外，良好提交说明，可以提前让评审者感知到改动点和改动影响。比如下面：

评审者从描述中就能确定我们改动的地方和改动的影响，从而可让评审者更快的进入状态，而不需要自己去阅读详细内容猜我们改了什么。

• 要点三：功能改动和非功能改动尽量分开
  

在一次功能中，我们除了功能性的改动，可能还包含一些非功能性的变动，如果都揉和在一起，往往看起来非常的难受。比如下面这种情况：

在这个改动中，我除了做一些功能上改动，还顺便修改了文档、对以往的格式问题做了格式化。虽然都是同一个改动，但观感还是非常差的。就像老师讲课，好的老师一定是按内容归纳，按内容教授，而不是代数几何混在一起讲。

因此我们可以在评审的提交中做进一步的拆分。还是针对上面的例子。我们调整后的样子为：

在评审页面的左侧点击全部提交，通过提交信息选择我们需要独立评审的提交内容。

选择功能性改动的提交，进行功能改动的评审。

选择非功能性提交，便可以只关注非功能的改动。比如这里我们只用单独看下README里的改动就可以，全程清爽。

保证测试基线

在很多已经成型的项目中，往往都有充分的测试覆盖扫描。当我们提交代码的时候，在功能上我们第一个要保证的就是以往的测试都能通过（除非是这个测试用例已经废弃或存在错误，这种我们应该单拎一个功能来修正），这也是对评审者的尊重，因为连功能的正确性都无法保证的评审是无意义的。

在云效中，我们可以将评审的目标分支作为保护分支，在自动化检查中配置自动化检查。通过在流水线中通过灵活的配置来设置我们工程中需要卡点检查。

然后，在我们提交的评审的时候，就会自动触发我们配置的卡点了。如果卡点不通过，是不允许评审通过的。因此我们在提交评审中，要注意卡点的内容，要让我们的评审“绿”起来。

评审中

重点：

• 积极对待评审反馈
• 代码是最好的解释
• 原谅评审人的误解/失误
• 保证Code Review的时效性

积极对待评审反馈

一部分同学反感代码评审的原因是，他们认为评审人是有意为难自己。不能排除在一些比较复杂的社会环境中确实可能存在这种问题。但是作为被评审人，我们还是要积极的看待评审的反馈。毕竟鸡蛋里挑骨头的前提是要有骨头，问题提出就一定有其合理性。绝大部分技术人对待代码还是中立的，不能积极面对评审更多的是我们担心自己的代码不够好，怕别人指出问题。但换个角度想，评审人其实也是在帮助我们，早点在评审阶段发现问题，总比线上出了问题要好。另外，在不断的评审中，我们也能快速的进步自己的编码能力。

代码是最好的解释

当评审者对我们的代码提出疑问的时候，我们除了解释，更应该做的是考虑为什么会出现这种疑问。有时候虽然我对评审人的疑问做了详细的回复，评审人似乎也理解了。但这个评审真的就结束了吗？马克思老人家说，我们应该透过现象看本质。毕竟评审人可能不只一个，即便当前的其他评审人看到这个解释也能明白,但我们无法保证后续涉及这块的代码改动再次评审的时候，会有人能明白这里的歧义。因此这种存在歧义的评审质疑，其本身的问题是代码未能做很好的解释。

面对这种情况，一般的建议添加充分的代码注释。但我更建议我们直接对代码进行重构，让代码本身就能解释清楚其意义。

原谅评审人的误解/失误

孔子云：人非圣贤孰能无过。评审者在评审中也会存在对正确代码的误解。在这种时候，我们不能得理不饶人，应当保持谦逊。正如在开篇中提到的，评审者实际是对我们提供帮助的，帮消灭我们自己未知的隐患。因此我们不应该贸然反击，应当理性回应，在解释的同时也是对我们自己的代码的一个很好的回顾。除此之外，我们还应意识到这里另一个问题——  存在即合理 ，如果评审者提出了疑问，是否证明这里存在不合理性，可能是代码有歧义需要重构？

保证Code Review的时效性

评审也应该有时效性。因为评审人评审的代码，往往不是自己的参与的业务，评审周期拉的太长，可能会想不起来。因此在评审中，往往会设置一个提醒的钩子。配合钉钉的webhook接收，能尽早的感知评审的人的意见。

在修改结束后及时给予反馈。这样让评审者对评审还没“冷却”的时候，就能更快的继续评审，大家就能早早下班啦。

评审后

评审后，评审的内容会保存在平台上，后面可以随时查看回顾。除此之外，在评审后最重要的是选择一个合并方式。一般我会选择squash的模式进行合并。将评审中的所有提交合并为一个commit。这样合并到主干中，一个提交就是一个功能。注意，这个和前文的提交拆分并不是一回事。在提交评审阶段，我们为了方便评审，将提交拆为功能改动和非功能改动。但在合并阶段因为已经通过了评审。我们要将一个功能的改动放到一个提交中。

点击合并后，评审中的多次提交最终会压缩为一个commit合并到目标分支中。这样也就保证了一个功能一个commit的原则。在后续排查问题和线上回滚都会非常方便。

P.S. 合并方式应该依据各自团队对工程管理和开发模式的选择来决定，以上只是一个简单的例子。

总结

总结一下，如果让评审者爱上给我做评审，我会这么做：

1. 提交前认真审视自己的代码，拒绝低级错误
2. 按功能拆分提交，写好提交说明
3. 让所有的单侧、集测变绿通过，保证质量基线
4. 积极对待评审反馈
5. 用代码解释评审者的疑问
6. 正确对待评审者的误解/失误
7. 及时修正/反馈，保证评审的时效性
8. 评审后选择正确的合并方式

本文作者：陈博俊，阿里云技术专家，Git开源项目贡献者，负责阿里巴巴经济体代码平台及云效代码平台底层架构设计及运维开发。

---

长按识别上图二维码 立即报名

6大专场

27位海内外技术大咖

邀你共享效能盛宴

锁定6月23日-6月24日

9:00-12:00  14:00-18:00

我们期待与您共建研发效能美好未来！

原文链接：https://developer.aliyun.com/article/784273?

版权声明：本文内容由阿里云实名注册用户自发贡献，版权归原作者所有，阿里云开发者社区不拥有其著作权，亦不承担相应法律责任。具体规则请查看《阿里云开发者社区用户服务协议》和《阿里云开发者社区知识产权保护指引》。如果您发现本社区中有涉嫌抄袭的内容，填写侵权投诉表单进行举报，一经查实，本社区将立刻删除涉嫌侵权内容。