一、幕后故事 时光荏苒,岁月如梭(太文绉绉了,这不是我的风格) 今天我准备聊聊在GitHub上如何玩CodeReview。 突发奇想?心血来潮?不是。 咋回事呢?(对八卦不感兴趣的可以直接跳到下一节,但是我猜你会感兴趣。) 首先我是DevStream开源社区成员。 在五月份,又有3位活跃的优秀的牛X的Contributors正式加入DevStream开源社区,正式成为了社区Member! (看下面的红框框) 于是乎,加上三月份的4个老Member,DevStream社区就有7个社区Member了(社区Member区分于像我这种在思码逸上班的内部Member)! 7个疯狂输出的Members,外加接近20个Contributors,我和铁心两个人基本就只能看着pr笑笑,一边表示欣喜,一边表示review不过来了,应接不暇,废了废了 也就是说,是时候组织一个reviewer组织,拉着大家一起玩CodeReview流程了! 说到CodeReview流程,流程是啥?规范是啥?规则是啥?技巧是啥?xxxx?我能预想到reviewersteam这个事情落地之日会有一堆问题砸到我头上。好吧,我需要写一篇文章来聊聊这些事。二、踏上旅途 下面我们开始一次CodeReview之旅。1。抢票阶段:认领一个Review任务 开始一次review之前,首先咱得认领一个review任务。 怎样算成功认领?如下图,Reviewers里有你的头像,这是当前pr你就是reviewers之一,同时可以看到黄色bar里的一行字Thispullrequestiswaitingonyourreview。以及绿色的按钮Addyourreview。你可以点击这个Addyourreview开始一次CodeReview之旅。 那么怎么认领呢?可能你还想问我。 这个问题有答案,也没有答案。 因为你是reviewer之一,那么你就有权限自己点击Reviewers右边的齿轮按钮,然后指定自己是一个Reviewer。如果你不是一个合法的人reviewer,那么你得先成为reviewer(Ifyouwant)。2。持票上船:开始Review流程 点击Addyourreview按钮后,就进入到了网页版CodeReview页面,大致如下: 这里有很多值得探索的特性,比如:左边的文件树、文件树上方过滤commits的下拉框、右边的文件过滤、每个文件右上角的Viewed选择框、每发现一个新功能,你的review过程就会多简化一分。 关于这个页面,没有比官方文档更权威和详细地介绍了,我想我没有理由搬一次砖,大家点击链接进一步探索吧。 对于简单的修改,或许网页直接查看代码diff就足够了,类似这种变更级别的pr: 我们可以轻松判断这个修改是不是正确,然后选择进一步的操作,比如Approve: 3。下船休整:下载一个pr到本地Review 对于一个不能一眼看穿的人pr,比如对方没有附上详细的测试结果等信息来证明他的修改已经充分测试(充分测试的例子),这时候靠肉眼我们很难判断这个pr合入后会怎样,或者不借助IDE的能力我们甚至很难看懂一些复杂的修改,这时候就需要下载这个pr对应的代码到本地,然后用IDE来帮助review了。 以pr641为例,我们需要下载这个pr,这时只需要执行这样两条命令:gitfetchupstreampull641head:pr641gitcheckoutpr641 效果如下: 然后在IDE里打开项目,就能看到pr对应的代码了: 代码在手,天下我有! 这会你可以在本地仔细查看每一处代码细节,也可以在本地跑一下makebuidj8或gotest。。。。之类的命令来逐步打消自己内心的疑虑。当然,pr本身会触发GitHubactionsworkflows,这些基础的buildut流程其实本地不跑也能知道是不是有错误,我们可以直接在页面上看到(每个项目具体的ci逻辑不一样): 到这里,你就基本能够完全看懂一个pr对应的所有修改了,是放他过?还是拦下马?他的命运掌握在你的手里!三、移花接木:提交一个commit到别人的pr 可能有时候,你需要修改别人的pr。哥们,我建议你抽根烟冷静一下,再问自己一句:我真的必须去修改他的pr吗?这样做会不会被打? 一般情况下,我不建议你去修改别人的pr,尤其是不能保证你的修改一定正确的情况下。因为你别人的pr本身就是容易冒犯到别人的事情,其次万一你改了之后发现还需要别人自己补一个commit,他会在复杂的git操作中骂你一万遍。 什么时候需要去动别人的pr呢?举个例子,比如这个pr。 首先这是一个newcontributor提交的firstpr,所以我不希望他的firstpr之旅太艰辛。然后这个feature其实并不简单,虽然技术上看并不难,但是要做到不重不漏就需要对dtm命令的所有子命令都了然于胸才行。显然,这对一个newcontributor来说要求太高了。所以在他提交了一个commit之后,我直接在这个commit的基础上又加了一个commit,完成了剩下的工作,并且在认可他的工作后告知其为什么我要修改这个pr,并附上了测试结果等。 具体怎么操作呢?步骤如下:修改代码,本地commit 前面我们已经下载了一个别人的pr到本地,接着自然是继续修改,然后提交commit(gitaddxxxgitcommitsmxxx),到这里都没啥技术含量,不赘述了。找到pr对应的源分支 在pr页面可以看到具体pr是从哪里提交的,比如: 我们点进去,就可以找到fork项目的地址信息,像这样: 于是,我们可以这样来加一个remote:gitremoteaddhimkugitgithub。com:himkudevstream。git 此时这个pr对应的fork项目的地址是himku(gitgithub。com:himkudevstream。git),对应分支是fixissue559,于是我们可以这样将自己新增的commit(s)提交到这个pr里(本地commit后执行):gitpushhimkuHEAD:fixissue559 是不是很酷?三、乱七八糟的规则:目的是啥?规范是啥?要求是啥?啥是啥? 再聊聊剩下的一些零零碎碎的问题,比如可能你想问review的目的啊,规范啊,要求啊,啊啊啊1。目的是啥? 可能你会问我为什么要codereview?我希望你别问。因为我不想总结。(这个问题可以Google到一堆答案) 我相信你心中有答案,codereview对应的是一堆的褒义词,比如:发现错误、保证质量、互相学习 你想的都是对的,总之这个事情值得做就对了,不需要去总结为什么。 啥? 你还是想听我谈谈? 谈谈就谈谈。软件质量:首先大多数的错误可以在codereview阶段被暴露出来,这些错误留到日后爆雷再被修复,代价会大很多,不管是造成的业务负面影响,还是额外付出的修复时间。所以codereview看似多花了时间,其实整体看是提升交付效率的;代码质量:严格执行codereview流程一方面可以互相督促对方:你别随便提交垃圾代码上来,会有人review;一方面假如有垃圾代码被提交上来了,可以有人站出来及时制止。完善的codereview流程可以避免代码可读性日益变差,维护成本日益增大,逐步变成屎山;传播经验:如果我写的代码很漂亮,我希望你来review,这时候一方面我想秀,无需避讳;另外一方面我希望你能够学着点,这是为你好;如果我的代码写的很烂,我希望你来review,这时候我希望你可以给我指出问题,帮助我提升编码水平,这是为我好;总之互相review,互相学习,你好我好大家好;2。规范与要求是啥? 当我们解决了所有流程或技巧层面的问题后,下一个非技术性问题是:怎样的代码需要返工?怎样的代码可以被合入? 我相信你心中会有这样的疑问。 对于有错误的代码,毫无疑问,不能合入,这不在我们的讨论范围。 那么正常运行,没有逻辑错误的代码呢?比如代码风格有点混乱,比如缺乏必要的一些注释,比如可读性差 我们分三个层次来思考:严格:我们完全可以指出任何是问题的问题,因为一份WTF的代码被合入后,所有对coder的骂声都包含一句潜台词reviewer干啥吃的?所以很简单,你觉得有问题的地方都可以提出来,包括:代码太复杂,看起来费力,我觉得可读性不好。我看了十分钟还是看不懂,说明可读性不够好。这个函数太长了,我鼠标滚了好几下才看完,你应该拆分一下。这个函数从名字我看不出来功能,但是我懒得看具体逻辑,为什么没有更多的注释?你这个源文件都五百行了,你要不拆分一下?这个包的逻辑很关键,你应该加点UT? 一般:如果一份代码功能完全正确,可读性也勉强还行,或许没有很好的面向对象来组织,或者注释不太详细,或者存在其他一些小小的不完美。这时候你也可以选择通过,避免太严格的review规则把一个pr的合入周期拖的太长,一方面影响交付效率,一方面打击开发者信心。很多时候我们可以对自己,或者对核心开发者要求更严格一些;但是对于社区贡献者,往往需要更多的宽容与认可。温柔:如果是一个newcontributor提交的一个firstpr,这时你可以放下各种能放下的要求,只要这份代码还过得去,就能合入,没有啥比给新人一些信心更重要的。如果pr存在一些小问题,你觉得对他来说改起来不会太困难,你可以留言友好的告诉他哪里需要改,怎么改;如果你觉得对他来说进一步做到完美有难度,你也可以直接提一个fix的commit到这个pr里,直接帮他修复问题,然后留个言告知他没有考虑到的问题是什么。 文章来自https:www。cnblogs。comdanielhutaopcodereview。html