在 GitHub 上玩转开源项目的 Code Review

daniel-hutao · · 1378 次点击 · · 开始浏览    
这是一个创建于 的文章,其中的信息可能已经有所发展或是发生改变。

## 一、幕后故事 时光荏苒,岁月如梭…… (🤮太文绉绉了,这不是我的风格) 今天我准备聊聊在 GitHub 上如何玩 Code Review。 突发奇想?心血来潮?不是。 咋回事呢?(对八卦不感兴趣的可以直接跳到下一节,但是我猜你会感兴趣。) --- 首先我是 DevStream 开源社区成员。 在五月份,又有3位活跃的优秀的牛X的 Contributors 正式加入 DevStream 开源社区,正式成为了社区 Member! (看下面的红框框) ![members.png](https://static.golangjob.cn/220604/fb170111a5f2ddfed4c09758ee493009.png) 于是乎,加上三月份的4个“老 Member”,DevStream 社区就有7个“社区 Member”了(社区 Member 区分于像我这种在思码逸上班的内部 Member)! 7个疯狂输出的 Members,外加接近20个 Contributors,我和[铁心](https://github.com/IronCore864)两个人基本就只能看着 pr 笑笑,一边表示欣喜,一边表示 review 不过来了,“应接不暇”,废了废了…… 也就是说,是时候组织一个 reviewer 组,拉着大家一起玩 Code Review 流程了! 说到 Code Review 流程,流程是啥?规范是啥?规则是啥?技巧是啥?xxxx?我能预想到 reviewers team 这个事情落地之日会有一堆问题砸到我头上。好吧,我需要写一篇文章来聊聊这些事。 ## 二、踏上旅途 下面我们开始一次 Code Review 之旅。 ### 1. 抢票阶段:认领一个 Review 任务 开始一次 review 之前,首先咱得“认领”一个 review 任务。 怎样算成功认领?如下图,Reviewers 里有你的头像,这时当前 pr 你就是 reviewers 之一,同时可以看到黄色 bar 里的一行字“*This pull request is waiting on your review.*”以及绿色的按钮“**Add your review**”。你可以点击这个“**Add your review**”开始一次 Code Review 之旅。 ![reviewer.png](https://static.golangjob.cn/220604/1b6dfe0bc13b818bce0691cd00a01932.png) “**那么怎么认领呢?**” 可能你还想问我。 这个问题有答案,也没有答案。 因为你是 reviewer 之一,那么你就有权限自己点击 Reviewers 右边的⚙️齿轮按钮,然后指定自己是一个 Reviewer。如果你不是一个“合法”的 reviewer,那么你得先成为 reviewer (If you want)。 ### 2. 持票上船:开始 Review 流程 点击 **Add your review** 按钮后,咱就进入到了网页版 Code Review 页面,大致如下: ![reviewpage.png](https://static.golangjob.cn/220604/1d4423a4bd954b06c96f790ddbb497ea.png) 这里有很多值得“探索”的特性,比如:左边的“文件树”、文件树上方过滤“commits”的下拉框、右边的“文件过滤”、每个文件右上角的 Viewed 选择框、…… 每发现一个新功能,你的 review 过程就会多简化一分。 关于这个页面,没有比[官方文档](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request)更权威和详细的介绍了,我想我没有理由“搬一次砖”,大家点击[链接](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request)进一步探索吧。 对于简单的修改,或许网页直接查看代码 diff 就足够了,类似这种变更级别的 pr: ![pr1.png](https://static.golangjob.cn/220604/9f85e29c89db0bf85cc78e9e4dd91dc3.png) 我们可以轻松判断这个修改是不是“正确”,然后选择进一步的操作,比如 Approve: ![lgtm.png](https://static.golangjob.cn/220604/aed31faaa59c0a2dd402cfb7daf56069.png) ### 3. 下船休整:下载一个 pr 到本地 Review 对于一个不能“一眼看穿”的 pr,比如对方没有附上详细的测试结果等信息来证明他的修改已经“充分测试”([充分测试的例子](https://github.com/devstream-io/devstream/pull/405)),这时候靠肉眼我们很难判断这个 pr 合入后会怎样,或者不借助 IDE 的能力我们甚至很难看懂一些复杂的修改,这时候就需要下载这个 pr 对应的代码到本地,然后用 IDE 来帮助 review 了。 以 [pr #641](https://github.com/devstream-io/devstream/pull/641)为例,我们需要下载这个 pr,这时只需要执行这样两条命令: ```sh git fetch upstream pull/641/head:pr641 git checkout pr641 ``` 效果如下: ![pr641.png](https://static.golangjob.cn/220604/366b99184475148a7ab140919ea69abd.png) 然后在 IDE 里打开项目,就能看到 pr 对应的代码了: ![ide.png](https://static.golangjob.cn/220604/6d5acdc776e08c9111eee786675ca6eb.png) “**代码在手,天下我有!**” 这会你可以在本地仔细查看每一处代码细节,可以在本地跑一下 `make buid -j8` 或 `go test ./...` 之类的命令来逐步“打消自己内心的疑虑”。当然,pr 本身会触发 GitHub actions workflows,这些基础的 *build/ut* 流程其实本地不跑也能知道是不是有错误,我们可以直接在页面上看到(每个项目具体的 ci 逻辑不一样): ![check.png](https://static.golangjob.cn/220604/13a2c54a80cb10527a535261290d8eed.png) 到这里,你就基本能够完全看懂一个 pr 对应的所有修改了,是放他过?还是拦下马?他的“命运”掌握在你的手里! ## 三、移花接木:提交一个 commit 到别人的 pr 可能有时候,你需要修改别人的 pr。哥们,我建议你抽根烟冷静一下,再问自己一句:我真的必须去修改他的 pr 吗?这样做会不会被打? 一般情况下,我不建议你去修改别人的 pr,尤其是不能保证你的修改一定正确的情况下。因为你别人的 pr 本身就是容易冒犯到别人的事情,其次万一你改了之后发现还需要别人自己补一个 commit,他会在复杂的 git 操作中骂你一万遍。 什么时候需要去动别人的 pr 呢?举个例子,比如[这个 pr](https://github.com/devstream-io/devstream/pull/589)。 首先这是一个 new contributor 提交的 first pr,所以我不希望他的 first pr 之旅太艰辛。然后这个 feature 其实并不简单,虽然技术上看并不难,但是要做到“不重不漏”就需要对 `dtm` 命令的所有子命令都“了然于胸”才行。显然,这对一个 new contributor 来说要求太高了。所以在他提交了一个 commit 之后,我直接在这个 commit 的基础上又加了一个 commit,完成了剩下的工作,并且在认可他的工作后告知其为什么我要修改这个 pr,并附上了测试结果等。 ![pr589.png](https://static.golangjob.cn/220604/d6e58b8d3a1e9200769082217bff4040.png) 具体怎么操作呢?步骤如下: 1. **修改代码,本地 commit** 前面我们已经下载了一个别人的 pr 到本地,接着自然是继续修改,然后提交 commit(`git add xxx & git commit -s -m "xxx"`),到这里都没啥技术含量,不赘述了。 2. **找到 pr 对应的源分支** 在 pr 页面可以看到具体 pr 是从哪里提交的,比如: ![branch.png](https://static.golangjob.cn/220604/77e9f48e0f82ae69ba63bcd4145d619e.png) 我们点进去,就可以找到 fork 项目的地址信息,像这样: ![fork.png](https://static.golangjob.cn/220604/2007430267a02ec00379dae52db07f7c.png) 于是,我们可以这样来加一个 remote: ```sh git remote add himku git@github.com:himku/devstream.git ``` 此时这个 pr 对应的 fork 项目的地址是 `himku`(git@github.com:himku/devstream.git),对应分支是 `fix-issue-559`,于是我们可以这样将自己新增的 commit(s) 提交到这个 pr 里(本地 commit 后执行): ```sh git push himku HEAD:fix-issue-559 ``` ![push.png](https://static.golangjob.cn/220604/3e30133d3b23e394b95fac4bfad99ae4.png) 是不是很酷? ## 三、乱七八糟的规则:目的是啥?规范是啥?要求是啥?啥是啥? 再聊聊剩下的一些零零碎碎的问题,比如可能你想问 review 的目的啊,规范啊,要求啊,啊啊啊…… ### 1. 目的是啥? 可能你会问我为什么要 code review ?我希望你别问。因为我不想总结。(这个问题可以 Google 到一堆答案) 我相信你心中有答案,code review 对应的是一堆的“褒义词”,比如:发现错误、保证质量、互相学习…… 你想的都是对的,总之这个事情值得做就对了,不需要去总结为什么。 啥? 你还是想听我谈谈? 谈谈就谈谈。 - **软件质量**:首先大多数的错误可以在 code review 阶段被暴露出来,这些错误留到日后“爆雷”再被修复,代价会大很多,不管是造成的业务负面影响,还是额外付出的修复时间。所以 code review 看似多花了时间,其实整体看是提升交付效率的; - **代码质量**:严格执行 code review 流程一方面可以互相督促对方:你别随便提交“垃圾”代码上来,会有人 review;一方面假如有“垃圾”代码被提交上来了,可以有人站出来及时制止。完善的 code review 流程可以避免代码可读性日益变差,维护成本日益增大,逐步变成“屎山”; - **传播经验**:如果我写的代码很漂亮,我希望你来 review,这时候一方面我想“秀”,无需避讳;另外一方面我希望你能够学着点,这是为你好;如果我的代码写的很烂,我希望你来 review,这时候我希望你可以给我指出问题,帮助我提升编码水平,这是为我好;总之互相 review,互相学习,你好我好大家好; ### 2. 规范与要求是啥? 当我们解决了所有“流程”或“技巧”层面的问题后,下一个“非技术性”问题是:**怎样的代码需要“返工”?怎样的代码可以被合入?** 我相信你心中会有这样的疑问。 对于有“错误”的代码,毫无疑问,不能合入,这不在我们的讨论范围。 那么正常运行,没有逻辑错误的代码呢?比如“代码风格有点混乱”,比如“缺乏必要的一些注释”,比如“可读性差”…… 我们分三个层次来思考: - **严格**:我们完全可以指出任何是“问题”的问题,因为一份 WTF 的代码被合入后,所有对 coder 的骂声都包含一句潜台词“reviewer 干啥吃的?”所以很简单,你觉得有问题的地方都可以提出来,包括: - “代码太复杂,看起来费力,我觉得可读性不好。” - “我看了十分钟还是看不懂,说明可读性不够好。” - “这个函数太长了,我鼠标滚了好几下才看完,你应该拆分一下。” - “这个函数从名字我看不出来功能,但是我懒得看具体逻辑,为什么没有更多的注释?” - “你这个源文件都五百行了,你要不拆分一下?” - “这个包的逻辑很关键,你应该加点 UT?” - …… ![wtf.png](https://static.golangjob.cn/220604/61115798fe2e65b4d5047b6a27a97238.png) - **一般**:如果一份代码功能完全正确,可读性也勉强还行,或许没有很好的面向对象来组织,或者注释不太详细,或者存在其他一些小小的不完美。这时候你也可以选择通过,避免太严格的 review 规则把一个 pr 的合入周期拖的太长,一方面影响交付效率,一方面打击开发者信心。很多时候我们可以对自己,或者对核心开发者要求更严格一些;但是对于社区贡献者,往往需要更多的“宽容”与认可。 - **温柔**:如果是一个 new contributor 提交的一个 first pr,这时你可以放下各种能放下的要求,只要这份代码还过得去,就能合入,没有啥比给新人一些信心更重要的。如果 pr 存在一些小问题,你觉得对他来说改起来不会太困难,你可以留言友好的告诉他哪里需要改,怎么改;如果你觉得对他来说进一步做到“完美”有难度,你也可以直接提一个 fix 的 commit 到这个 pr 里,直接帮他修复问题,然后留个言告知他没有考虑到的问题是什么。 ## 终点站到了,下船! 今天就聊到这里。 意犹未尽? 再去看看 Google 的 [Code Review Developer Guide](https://google.github.io/eng-practices/review/) 吧! --- - 你可以收藏我的个人网站 <https://www.danielhu.cn>,阅读更多我写的博客文章; - 你也可以关注我的个人微信公众号“胡说云原生”,第一时间接收新文章推送; - 当然,你也可以收藏 DevStream 的博客网站 <https://blog.devstream.io> 或者官网 <https://www.devstream.io>,里面不止有我这种“不严肃”的人发的“主要用来搞笑”的文章,还有 DevStream 团队其他成员发的“严肃讲知识”的“科普文”。

有疑问加站长微信联系(非本文作者))

入群交流(和以上内容无关):加入Go大咖交流群,或添加微信:liuxiaoyan-s 备注:入群;或加QQ群:692541889

1378 次点击  
加入收藏 微博
被以下专栏收入,发现更多相似内容
暂无回复
添加一条新回复 (您需要 登录 后才能回复 没有账号 ?)
  • 请尽量让自己的回复能够对别人有帮助
  • 支持 Markdown 格式, **粗体**、~~删除线~~、`单行代码`
  • 支持 @ 本站用户;支持表情(输入 : 提示),见 Emoji cheat sheet
  • 图片支持拖拽、截图粘贴等方式上传