如何用“测试”和“提交单一性” 提高PR效率

业务总是在慢慢变得复杂,常常是一个同事负责的某一块,另一个同事对此并不能全盘理解,但一个产品组内同事们需要互相 review 代码以其能尽早发现产品问题,避免将问题留给客户来报告。

如何做好 code review 以及 code review 的时候有哪些要点?靠人力一行一行在颅内运行 PR 的代码,很消耗注意力。依靠人判断代码的正确性是不靠谱的。因为“人”本身就很容易犯错。应该靠机制,而不是靠人力来保证代码的鲁棒性,应该使用科学且合理的机制来确保 pull request 的正确与质量

参考开源项目的做法,发送 PR 的时候代码服务器首先检查是否有冲突,接着它会先将本次 PR 在一个临时的地方合并,然后运行对应的所有自动化测试,如果有测试失败,则拒绝本次 PR。这样极大的促进了 PR 的效率,对于发起者来说,可以尽早知道自己触发的问题,对于 管理者,只有本次 PR 是”正确“的才会收到 PR 通知

提高 PR 正确性的关键就是 “测试”。但由于部分产品的代码存在历史负担,无法轻易的加入 自动化测试。但通过一点点改进还是可以享受到“测试”的益处。 那就是确保每次”提交单一性“,即每次 commit 或者 PR只包含一个功能修改。一次提交只包含一件事 增加一个新的 feature,code refine,或者 bug fix。针对本次 PR 最好能提供对应的测试代码或者验证手段(包含验证远离和过程)

“测试”,对于 code review很重要,它可以为 PR 提供一种验证方式。可以使用 testcase 也可以是验证你这个 PR 正确性的一个脚本或者是一种操作顺序用以证明这次PR包含的变动达到了预期的效果

举个例子,服务端有一个 login 功能,接受 username 和 password 用来处理用户登录请求。
客户端程序在某一次版本升级过程中不小心修改了登录相关的代码没有注意到,导致有的客户在重新登录的时总是无法成功。你查看堆栈后发现是客户端在提交 login 参数的时候,修改了对于 usernamepassword base64 的方式。原先是 base64 no padding,你在上一次修改其它接口时全局修改成了base64 padding

弄清楚问题之后,你很快修改好了代码,并且本地测试OK。此时的要实现的“测试”可以是在 testcase 里加入两个情况:
一是还原本次问题的测试,在这个测试里应该还原出客户遇到的问题堆栈(注意在注视里描述本次bug情况)
一是解决之后的测试,在这次测试里验证了修改后的代码可以正常work

如果你觉得 testcase 写起来太过麻烦,那也没必要拘泥于传统,你可以写一个 shell 脚本,假设你起名叫做 issue-8090-client-login-failed-snippet.sh 在这个脚本中,用 curl 来模拟两种情况下的登录请求,一种是 bug 代码实际发出的请求,一种是修复后代码发出的登录请求。
主要思路就是提供一种“清晰的”、“可重复的”验证手段,确保你的 reviewer 能够在 ta 本地很方便的验证你的PR。

你可能在修改了这个bug之后发现有几个变量或者函数明明不够“优雅”,想顺手 refine 一波,但“提交单一性”原则在此处提醒你,将本次 PR 单独发出,主要是方便reviewer 审阅你的PR,你可以有以下两个提交,并且应该做成两个 PR,具体的命令行大概是这样
假设当前在 master 分支
第一个 PR 的步骤

1
2
3
$ git checkout -b issue-8090-fix-bug
# make some changes to fix the bug and add necessary test case
$ git commit -m “issue-8090 fix login bug by using correct base64 method”

第二个 PR 的步骤

1
2
3
4
$ git checkout master  
$ git checkout -b issue-8090-refine
# do your refine job
$ git commit -m "issue-8090 fix login issue by using correct base64 style"

上面举的这个例子比较简单,条件也比较单一,现实中我们做的工作往往比这个复杂,但我们自己写起来也是一步一个脚印,不断解决问题,可能在做的过程中每一步你都知道为什么,但时间久了再回想起来可能自己都想不起来当时为什么要这么写。

现在,站在代码合并者的角度想想,ta在 review 你的代码过程中忍受的是怎样的一种煎熬?

ta 可能在想,这是啥,这又是啥,你这里为什么要这样写,你这样写能 work 吗?

虽然在 code review 的评论过程能彼此理解,但这极大的消耗了双方的时间,也降低了整体的开发效率。最严重的是,一个 PR 经过多次”扯皮“之后会让人产生焦虑。当这样 code review 的次数多了以后,其消耗的总体时间一定是 远远大于 发起 PR 的人编写 testcase 的时间的。