关于github:在-GitHub-上玩转开源项目的-Code-Review

一、幕后故事

时光荏苒，岁月如梭……（🤮太文绉绉了，这不是我的格调）

明天我筹备聊聊在 GitHub 上如何玩 Code Review。

突发奇想？灵机一动？不是。

咋回事呢？（对八卦不感兴趣的能够间接跳到下一节，然而我猜你会感兴趣。）

首先我是 DevStream 开源社区成员。

在五月份，又有 3 位沉闷的优良的牛 X 的 Contributors 正式退出 DevStream 开源社区，正式成为了社区 Member！

(看上面的红框框)

于是乎，加上三月份的 4 个“老 Member”，DevStream 社区就有 7 个“社区 Member”了（社区 Member 辨别于像我这种在思码逸下班的外部 Member）！

7 个疯狂输入的 Members，外加靠近 20 个 Contributors，我和铁心两个人根本就只能看着 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 之一，那么你就有权限本人点击 Reviewers 左边的⚙️齿轮按钮，而后指定本人是一个 Reviewer。如果你不是一个“非法”的 reviewer，那么你得先成为 reviewer (If you want)。

2. 持票上船：开始 Review 流程

点击 Add your review 按钮后，咱就进入到了网页版 Code Review 页面，大抵如下：

这里有很多值得“摸索”的个性，比方：右边的“文件树”、文件树上方过滤“commits”的下拉框、左边的“文件过滤”、每个文件右上角的 Viewed 抉择框、…… 每发现一个新性能，你的 review 过程就会多简化一分。

对于这个页面，没有比官网文档更权威和具体的介绍了，我想我没有理由“搬一次砖”，大家点击链接进一步摸索吧。

对于简略的批改，或者网页间接查看代码 diff 就足够了，相似这种变更级别的 pr：

咱们能够轻松判断这个批改是不是“正确”，而后抉择进一步的操作，比方 Approve：

3. 下船休整：下载一个 pr 到本地 Review

对于一个不能“一眼看穿”的 pr，比方对方没有附上具体的测试后果等信息来证实他的批改曾经“充沛测试”（充沛测试的例子），这时候靠肉眼咱们很难判断这个 pr 合入后会怎么，或者不借助 IDE 的能力咱们甚至很难看懂一些简单的批改，这时候就须要下载这个 pr 对应的代码到本地，而后用 IDE 来帮忙 review 了。

以 pr #641 为例，咱们须要下载这个 pr，这时只须要执行这样两条命令：

git fetch upstream pull/641/head:pr641
git checkout pr641

成果如下：

而后在 IDE 里关上我的项目，就能看到 pr 对应的代码了：

“ 代码在手，天下我有！”

这会你能够在本地认真查看每一处代码细节，能够在本地跑一下 make buid -j8 或 go test ./... 之类的命令来逐渐“打消本人心田的疑虑”。当然，pr 自身会触发 GitHub actions workflows，这些根底的 build/ut 流程其实本地不跑也能晓得是不是有谬误，咱们能够间接在页面上看到（每个我的项目具体的 ci 逻辑不一样）：

到这里，你就根本可能齐全看懂一个 pr 对应的所有批改了，是放他过？还是拦上马？他的“命运”把握在你的手里！

三、偷梁换柱：提交一个 commit 到他人的 pr

可能有时候，你须要批改他人的 pr。哥们，我倡议你抽根烟沉着一下，再问本人一句：我真的必须去批改他的 pr 吗？这样做会不会被打？

个别状况下，我不倡议你去批改他人的 pr，尤其是不能保障你的批改肯定正确的状况下。因为你他人的 pr 自身就是容易触犯到他人的事件，其次万一你改了之后发现还须要他人本人补一个 commit，他会在简单的 git 操作中骂你一万遍。

什么时候须要去动他人的 pr 呢？举个例子，比方这个 pr。

首先这是一个 new contributor 提交的 first pr，所以我不心愿他的 first pr 之旅太艰苦。而后这个 feature 其实并不简略，尽管技术上看并不难，然而要做到“不重不漏”就须要对 dtm 命令的所有子命令都“了然于胸”才行。显然，这对一个 new contributor 来说要求太高了。所以在他提交了一个 commit 之后，我间接在这个 commit 的根底上又加了一个 commit，实现了剩下的工作，并且在认可他的工作后告知其为什么我要批改这个 pr，并附上了测试后果等。

具体怎么操作呢？步骤如下：

1. 批改代码，本地 commit

后面咱们曾经下载了一个他人的 pr 到本地，接着天然是持续批改，而后提交 commit（git add xxx & git commit -s -m "xxx"），到这里都没啥技术含量，不赘述了。

2. 找到 pr 对应的源分支

在 pr 页面能够看到具体 pr 是从哪里提交的，比方：

咱们点进去，就能够找到 fork 我的项目的地址信息，像这样：

于是，咱们能够这样来加一个 remote：

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 后执行）：

git push himku HEAD:fix-issue-559

是不是很酷？

三、乌七八糟的规定：目标是啥？标准是啥？要求是啥？啥是啥？

再聊聊剩下的一些零零碎碎的问题，比方可能你想问 review 的目标啊，标准啊，要求啊，啊啊啊……

1. 目标是啥？

可能你会问我为什么要 code review？我心愿你别问。因为我不想总结。（这个问题能够 Google 到一堆答案）

我置信你心中有答案，code review 对应的是一堆的“褒义词”，比方：发现错误、保证质量、互相学习……

你想的都是对的，总之这个事件值得做就对了，不须要去总结为什么。

啥？

你还是想听我谈谈？

谈谈就谈谈。

• 软件品质 ：首先大多数的谬误能够在 code review 阶段被裸露进去，这些谬误留到日后“爆雷”再被修复，代价会大很多，不论是造成的业务负面影响，还是额定付出的修复工夫。所以 code review 看似多花了工夫，其实整体看是晋升交付效率的；
• 代码品质 ：严格执行 code review 流程一方面能够相互督促对方：你别轻易提交“垃圾”代码上来，会有人 review；一方面如果有“垃圾”代码被提交上来了，能够有人站进去及时禁止。欠缺的 code review 流程能够防止代码可读性日益变差，保护老本日益增大，逐渐变成“屎山”；
• 流传教训 ：如果我写的代码很漂亮，我心愿你来 review，这时候一方面我想“秀”，无需避讳；另外一方面我心愿你可能学着点，这是为你好；如果我的代码写的很烂，我心愿你来 review，这时候我心愿你能够给我指出问题，帮忙我晋升编码程度，这是为我好；总之相互 review，互相学习，你好我好大家好；

2. 标准与要求是啥？

当咱们解决了所有“流程”或“技巧”层面的问题后，下一个“非技术性”问题是： 怎么的代码须要“返工”？怎么的代码能够被合入？

我置信你心中会有这样的疑难。

对于有“谬误”的代码，毫无疑问，不能合入，这不在咱们的探讨范畴。

那么失常运行，没有逻辑谬误的代码呢？比方“代码格调有点凌乱”，比方“不足必要的一些正文”，比方“可读性差”……

咱们分三个档次来思考：

• 严格 ：咱们齐全能够指出任何是“问题”的问题，因为一份 WTF 的代码被合入后，所有对 coder 的骂声都蕴含一句潜台词“reviewer 干啥吃的？”所以很简略，你感觉有问题的中央都能够提出来，包含：

  • “代码太简单，看起来费劲，我感觉可读性不好。”
  • “我看了十分钟还是看不懂，阐明可读性不够好。”
  • “这个函数太长了，我鼠标滚了好几下才看完，你应该拆分一下。”
  • “这个函数从名字我看不出来性能，然而我懒得看具体逻辑，为什么没有更多的正文？”
  • “你这个源文件都五百行了，你要不拆分一下？”
  • “这个包的逻辑很要害，你应该加点 UT？”
  • ……

• 个别 ：如果一份代码性能完全正确，可读性也勉强还行，或者没有很好的面向对象来组织，或者正文不太具体，或者存在其余一些小小的不完满。这时候你也能够抉择通过，防止太严格的 review 规定把一个 pr 的合入周期拖的太长，一方面影响交付效率，一方面打击开发者信念。很多时候咱们能够对本人，或者对外围开发者要求更严格一些；然而对于社区贡献者，往往须要更多的“宽容”与认可。
• 温顺 ：如果是一个 new contributor 提交的一个 first pr，这时你能够放下各种能放下的要求，只有这份代码还过得去，就能合入，没有啥比给新人一些信念更重要的。如果 pr 存在一些小问题，你感觉对他来说改起来不会太艰难，你能够留言敌对的通知他哪里须要改，怎么改；如果你感觉对他来说进一步做到“完满”有难度，你也能够间接提一个 fix 的 commit 到这个 pr 里，间接帮他修复问题，而后留个言告知他没有思考到的问题是什么。

终点站到了，下船！

明天就聊到这里。

意犹未尽？

再去看看 Google 的 Code Review Developer Guide 吧！

• 你能够珍藏我的集体网站 https://www.danielhu.cn，浏览更多我写的博客文章；
• 你也能够关注我的集体微信公众号“胡说云原生”，第一工夫接管新文章推送；
• 当然，你也能够珍藏 DevStream 的博客网站 https://blog.devstream.io 或者 …，外面不止有我这种“不庄重”的人发的“次要用来搞笑”的文章，还有 DevStream 团队其余成员发的“庄重讲常识”的“科普文”。