新聞中心
前幾天看了《Code Review 程序員的寄望與哀傷》,想到我們團(tuán)隊(duì)開展Code Review也有2年了,結(jié)果還算比較滿意,有些經(jīng)驗(yàn)應(yīng)該可以和大家一起分享、探討。
我們?yōu)槭裁匆菩蠧ode Review呢?我們當(dāng)時(shí)面臨著代碼混亂、Bug頻出的狀況。
當(dāng)時(shí)我覺得要有所改變,希望能提高產(chǎn)品的代碼質(zhì)量,改善開發(fā)團(tuán)隊(duì)面臨的困境。并且我個(gè)人在開發(fā)上有很多經(jīng)驗(yàn),也希望這些知識(shí)能夠在團(tuán)隊(duì)內(nèi)傳播。
各種考慮后,我們***認(rèn)為推行Code Review能改善或解決我們面臨的很多問題。

創(chuàng)新互聯(lián)專注于企業(yè)全網(wǎng)營(yíng)銷推廣、網(wǎng)站重做改版、競(jìng)秀網(wǎng)站定制設(shè)計(jì)、自適應(yīng)品牌網(wǎng)站建設(shè)、html5、商城網(wǎng)站建設(shè)、集團(tuán)公司官網(wǎng)建設(shè)、外貿(mào)網(wǎng)站制作、高端網(wǎng)站制作、響應(yīng)式網(wǎng)頁(yè)設(shè)計(jì)等建站業(yè)務(wù),價(jià)格優(yōu)惠性價(jià)比高,為競(jìng)秀等各大城市提供網(wǎng)站開發(fā)制作服務(wù)。
這篇文章的目的不是告訴大家怎么在一個(gè)團(tuán)隊(duì)內(nèi)推行Code Review,首先因?yàn)槲覀€(gè)人僅在一家公司內(nèi)推行過,并沒有很多經(jīng)驗(yàn)。
其次每家公司、每個(gè)團(tuán)隊(duì)的情況都不太一樣,應(yīng)該根據(jù)公司或團(tuán)隊(duì)的實(shí)際情況選擇恰當(dāng)?shù)姆桨?,并根?jù)成員的反饋來及時(shí)調(diào)整,推動(dòng)Code Review的實(shí)施。
所以,本文是介紹我們公司是如何實(shí)施Code Review的,我們是如何解決我們遇到的問題的,希望我們的經(jīng)驗(yàn)?zāi)芙o大家?guī)硇椭?br /> 行文倉(cāng)促,如有遺漏或錯(cuò)誤,歡迎指正。
***程和規(guī)則
經(jīng)過簡(jiǎn)單的對(duì)比、試用,我們***采用了Git Flow+Pull Request(PR)模式來做Code Review。(PR模式詳情可參見 Git工作流指南:Pull Request工作流)
Pull Request(PR)簡(jiǎn)單的說就是你沒有權(quán)限往一個(gè)特定的倉(cāng)庫(kù)或分支提交代碼,你請(qǐng)求有權(quán)限的人把你提交的代碼從你的倉(cāng)庫(kù)或分支合并到指定的倉(cāng)庫(kù)或分支。
由于PR需要有權(quán)限的人確認(rèn),所以非常適合在這個(gè)過程中做Code Review,是否接受或者拒絕就取決于Code Review的結(jié)果。
在支持PR模式的軟件里,每一個(gè)PR都有一個(gè)新增代碼的對(duì)比(diff)界面。
代碼審核者可以在線瀏覽請(qǐng)求合并的新增代碼,并針對(duì)有疑問的代碼行添加評(píng)論,通過這種方式來實(shí)現(xiàn)Code Review。
評(píng)論可以被所有有權(quán)限查看倉(cāng)庫(kù)的人看到,每個(gè)人都可以回復(fù)任何人的評(píng)論,有點(diǎn)像論壇里某個(gè)帖子的討論。
這種模式是事后審核,也就是代碼已經(jīng)提交到了中心倉(cāng)庫(kù),Review過程中頻繁的改動(dòng)會(huì)造成歷史簽入記錄的混亂。
當(dāng)然Git可以采用更改歷史記錄來解決這個(gè)問題,由于容易誤操作,我們一般只在基礎(chǔ)類庫(kù)這類要求比較嚴(yán)格的項(xiàng)目上實(shí)施。
我們所了解到的支持PR模式的軟件都采用Git作為源代碼版本控制工具,所以我們的源代碼版本控制工具也遷移到了Git。
由于Git太靈活了,因此誕生了很多的Git流程,用來規(guī)范Git的使用。
常見的有集中式工作流、功能分支工作流、Gitflow工作流、Forking工作流、Github工作流。
我們對(duì)Git Flow做了些調(diào)整,調(diào)整后的流程被命名為Baza Flow,定義見后文。
根據(jù)Baza Flow,我們大部分倉(cāng)庫(kù)只定義了2個(gè)主干分支,master和develop。(例外,我們有一個(gè)倉(cāng)庫(kù)有3個(gè)開發(fā)小組同時(shí)進(jìn)行開發(fā),定義了4個(gè)主干分支,目前還比較順暢,再多估計(jì)主干分支之間的合并就比較繁瑣了。)
master對(duì)應(yīng)生產(chǎn)環(huán)境代碼,所有面向生產(chǎn)環(huán)境的發(fā)布來源都是master分支的代碼。develop則對(duì)應(yīng)本地測(cè)試環(huán)境的代碼。
絕大多數(shù)情況下,QA(測(cè)試)只測(cè)試develop分支和master分支的代碼。
由于開發(fā)人員都在一個(gè)團(tuán)隊(duì)內(nèi),所以我們沒有采用基于倉(cāng)庫(kù)的PR,采用的是基于分支的PR。
我們對(duì)主干分支的操作權(quán)限做了限制,只有特定的人才能操作,develop分支是項(xiàng)目開發(fā)Leader和架構(gòu)師,master分支是QA。
有權(quán)限往主干分支合并的成員會(huì)按照約定的規(guī)則來執(zhí)行合并,不會(huì)合并沒有完成審核的PR。
上面這點(diǎn)其實(shí)蠻重要的,所以我們會(huì)對(duì)有權(quán)限合并的人有特別的約定,在什么情況下才能合并代碼。(見后文PR的說明)
PR的發(fā)起人要主動(dòng)的推動(dòng)PR的審核,Leader也會(huì)密切關(guān)注PR審核的進(jìn)度,在需要的時(shí)候及時(shí)介入。
我們配置了CI服務(wù)器(什么是CI)只編譯特定的分支,通常是develop和master分支。
所有的代碼合并到了主干分支之后,都會(huì)自動(dòng)觸發(fā)編譯和本地測(cè)試環(huán)境的發(fā)布,QA無需依賴開發(fā)人員編譯的代碼來測(cè)試,也無需自己手工操作這些,保證了開發(fā)人員和測(cè)試人員的相互獨(dú)立。
我們本地測(cè)試環(huán)境的發(fā)布包含了數(shù)據(jù)庫(kù)和站點(diǎn)的發(fā)布,全自動(dòng)的,發(fā)布完成以后就是一個(gè)可用的產(chǎn)品,有時(shí)間這部分也可以分享一下。
我們還使用了Scrum里面一個(gè)很重要的概念:完成定義。
就是我們規(guī)定了我們一個(gè)任務(wù)的完成被定義為:代碼編寫完成,經(jīng)過自測(cè),提交的PR經(jīng)過審核并且合并到主干分支。
也就是說,所有的代碼被合并到了主干分支之后任務(wù)才算是完成,而被合并到主干分支必須要經(jīng)過Code Review,這是強(qiáng)制的。
Baza Flow
當(dāng)前版本 V0.9
Baza Flow 由 Git Flow 演化而來,Git Flow的開發(fā)模式如下圖所示:
由于我們的托管軟件對(duì)于Pull Request的限制,我們對(duì)Git Flow做了改動(dòng),改動(dòng)的地方有:
1、每一個(gè)大功能我們會(huì)創(chuàng)建一個(gè)單獨(dú)的feature分支,項(xiàng)目開發(fā)人員基于這個(gè)單獨(dú)的feature分支創(chuàng)建自己的任務(wù)分支。
比如,對(duì)于CS 2項(xiàng)目來說,啟動(dòng)的時(shí)候分支的創(chuàng)建是:master -> develop -> feature/v2。
開發(fā)人員應(yīng)該基于這個(gè)大特性分支feature/v2來創(chuàng)建自己的任務(wù)分支,比如創(chuàng)建XXXX,可以用一個(gè)單獨(dú)的分支feature/v2-xxxx。
完成這個(gè)任務(wù)以后,立即向上游分支(feature/v2)提交pull request。然后從feature/v2-xxxx 創(chuàng)建自己的下一個(gè)任務(wù)分支,比如YYYY編輯 feature/v2-yyyy。
請(qǐng)注意,合并到上游分支的功能必須相對(duì)獨(dú)立而且是可用的,分支任務(wù)工作量0.5-1個(gè)工作日,不宜超過2個(gè)工作日,超過2個(gè)工作日不向上游合并,需要向團(tuán)隊(duì)解釋。
代碼經(jīng)過Review以后,可能會(huì)進(jìn)行必要的修改,修改在原分支修改,修改完畢代碼合并進(jìn)上游分支,原分支會(huì)定期刪除。
項(xiàng)目組成員在收到合并成功的通知后,請(qǐng)自行從上游大特性分支向下合并到自己當(dāng)前的開發(fā)分支。
提交pull request后創(chuàng)建新任務(wù)分支的時(shí)候務(wù)必知會(huì)一下相關(guān)配合同事(比如前端的同事),讓他們?cè)谛碌姆种侠^續(xù)開發(fā)。
2、對(duì)于小功能,預(yù)計(jì)在0.5-1個(gè)(不超過2個(gè))工作日工作量的開發(fā)任務(wù),直接基于develop分支創(chuàng)建特性分支即可。
3、在各個(gè)分支遇到的bug,請(qǐng)基于該分支創(chuàng)建一個(gè)Bug分支。
如果在缺陷跟蹤管理系統(tǒng)上有對(duì)應(yīng)的項(xiàng),命名請(qǐng)使用缺陷跟蹤管理系統(tǒng)的ID,比如BAZABUG-1354 比如這個(gè)Bug的分支命名就是bugfix/BAZABUG-1354。
如果在缺陷跟蹤管理系統(tǒng)上沒有對(duì)應(yīng)的項(xiàng),命名請(qǐng)簡(jiǎn)短的說明修改內(nèi)容,比如“JX 9df2b01 引用bootstrap css虛擬路徑重寫,避免出現(xiàn)字體無法找到的問題”,分支命名可以是bugfix/miss-font。
完成修改以后提交并推送到中心倉(cāng)庫(kù)然后立即向上游分支提交pull request。
4、發(fā)起pull request以后,請(qǐng)將pull request的鏈接在IM上發(fā)給代碼審核者,以此通知對(duì)方及時(shí)進(jìn)行審核。
二、執(zhí)行
我們?cè)趫F(tuán)隊(duì)內(nèi)部提倡質(zhì)量?jī)?yōu)先,開發(fā)團(tuán)隊(duì)不能為了進(jìn)度犧牲質(zhì)量,并在團(tuán)隊(duì)內(nèi)部達(dá)成了共識(shí)。
所以,無論進(jìn)度有多么緊迫,Code Review的過程都一定會(huì)做。
所有的問題一定會(huì)被提出,只是會(huì)根據(jù)進(jìn)度的緊迫程度,以及問題的大小,改動(dòng)成本,決定問題是現(xiàn)在解決,還是加一個(gè)TODO,并記錄在缺陷跟蹤管理系統(tǒng)內(nèi),以防日后遺忘。
多數(shù)情況下,我們都會(huì)要求立即解決,哪怕因此造成了發(fā)布的推遲。
我們深知,其實(shí)多數(shù)情況下,現(xiàn)在不解決,日后不知道猴年馬月才能解決。
我們?cè)趫F(tuán)隊(duì)內(nèi)推行Code Review的過程中沒有遇到太多阻力。
原因大概有兩點(diǎn),首先管理層方面了解之前遇到的各種問題,也迫切希望能有所改善,所以從一開始就是支持的態(tài)度。
其次,絕大部分開發(fā)人員覺得在這個(gè)過程中能自己能學(xué)習(xí)到東西,并沒有抵觸,遇到很好的意見時(shí)大家都還是很高興的。
***,慢慢的形成了一種氛圍,整個(gè)團(tuán)隊(duì)都會(huì)自覺的維護(hù)它。
附一張我們審核的對(duì)話圖,這位童鞋嘗試對(duì)系統(tǒng)內(nèi)部散落各地發(fā)業(yè)務(wù)郵件的代碼做一個(gè)整理,用一套模式來處理,調(diào)整了3版才定調(diào),然后修改了很多細(xì)節(jié)才通過了合并,前后大概用一個(gè)多星期時(shí)間:
表面上看來Code Review會(huì)延緩項(xiàng)目的進(jìn)度,但是在我們2年多的執(zhí)行過程中,大多數(shù)時(shí)候沒感覺到有延緩。
原因是,雖然代碼合并的周期變長(zhǎng)了,但是由于代碼質(zhì)量提高了,導(dǎo)致Bug變少了,由于Bug引起的返工問題也變少了,因此整體的進(jìn)度其實(shí)并沒有延緩。
我個(gè)人認(rèn)為對(duì)一個(gè)成熟的團(tuán)隊(duì)其實(shí)做Code Review反而會(huì)加快整體的項(xiàng)目進(jìn)度,但是手頭上沒有統(tǒng)計(jì)數(shù)據(jù)支撐我的觀點(diǎn)。(對(duì)于軟件開發(fā)的度量,歡迎有心得的同學(xué)告知我)
我們每個(gè)分支有權(quán)限合并的人都不止一個(gè),這樣可以保證有人請(qǐng)假不在的時(shí)候,代碼仍然可以被其他同事審核通過之后合并。
半年前,我們團(tuán)隊(duì)加入了很多新成員,剛加入的新同事對(duì)規(guī)范、項(xiàng)目、產(chǎn)品的熟悉程度都不高,導(dǎo)致了有一段時(shí)間,我們遇到了PR審核周期變長(zhǎng)的問題。
加上之前遇到的一些問題,我們總結(jié)了一個(gè)說明,目的是減輕Code Review對(duì)開發(fā)人員工作的負(fù)擔(dān),加快PR審核通過的過程。
說明如下:
Pull Request 的說明
任務(wù)完成才能提交PR。
PR應(yīng)該在一個(gè)工作日內(nèi)被合并或者被拒絕。
PR在有嚴(yán)重問題(包括但不限于架構(gòu)問題、安全問題、設(shè)計(jì)問題),太多問題,或者任務(wù)無效的情況下會(huì)被拒絕。
嚴(yán)禁一個(gè)PR里面有多個(gè)任務(wù),除非它們是緊密關(guān)聯(lián)的。
PR提交之后只允許針對(duì)Review發(fā)現(xiàn)問題再次提交代碼,除非有充足的理由,嚴(yán)禁在同一個(gè)PR中再次提交其它任務(wù)的代碼。
提交PR時(shí)候有一個(gè)描述框,內(nèi)容會(huì)自動(dòng)根據(jù)Commit的message合并而成。
切記,如果一次提交的內(nèi)容包含很多Commit,請(qǐng)不要使用自動(dòng)生成的描述。
請(qǐng)用簡(jiǎn)短但是足夠說明問題的語言(理想是控制在3句話之內(nèi))來描述:
你改動(dòng)了什么,解決了什么問題,需要代碼審查的人留意那些影響比較大的改動(dòng)。
特別需要留意,如果對(duì)基礎(chǔ)、公共的組件進(jìn)行了改動(dòng),一定要另起一行特別說明。
審核人員邀請(qǐng)?jiān)瓌t:
1. 在創(chuàng)建PR時(shí),Reviewers(審核人)一欄里主要填寫“必需審核人”。只有這些人審核都通過,才允許合并。
2. 除了“必需審核人”外,還有一些其它審核人,我們可以在Description里做為“邀請(qǐng)審核嘉賓”@進(jìn)來。
3. 主干分支間的合并,如Develop => Master,或Master => Develop等,則需要把整個(gè)團(tuán)隊(duì)(開發(fā)+QA)都列為“必需審核人”。
必須審核人的列表由團(tuán)隊(duì)決定,可能包括以下人選:
團(tuán)隊(duì)Leader
- 前端架構(gòu)師(如果有前端代碼改動(dòng)) (可以授權(quán))
- 后端架構(gòu)師(如果有后端代碼改動(dòng)) (可以授權(quán))
- 產(chǎn)品架構(gòu)師
- 對(duì)此PR解決的問題比較熟悉的(之前一直負(fù)責(zé)這部分業(yè)務(wù)的同事)
- 此PR解決的問題對(duì)他影響比較大(比如認(rèn)領(lǐng)的任務(wù)依賴此PR的同事)
其它審核人,包括但不限于:
需要知悉此處代碼改動(dòng)的人但又不必非要其審核通過的同事
可以從這個(gè)PR中學(xué)習(xí)的同事
可以授權(quán)指的是,根據(jù)約定,Bug修復(fù)之類的改動(dòng),或者影響較小的改動(dòng),前端架構(gòu)師和后端架構(gòu)師可以授權(quán)團(tuán)隊(duì)內(nèi)的某個(gè)資深開發(fā)人員,由這個(gè)資深開發(fā)人員代表他們進(jìn)行審核。
主干分支之間的合并,大型Feature的合并,前端架構(gòu)師和后端架構(gòu)師需要參與。
上述審核人關(guān)注的視角不太一樣:
團(tuán)隊(duì)Leader關(guān)注你是否完成了任務(wù),前后端架構(gòu)師關(guān)注是否符合公司統(tǒng)一的架構(gòu)、風(fēng)格、質(zhì)量,產(chǎn)品架構(gòu)師從整個(gè)產(chǎn)品層面來關(guān)注這個(gè)PR。
熟悉此問題的同事可以更好的保證問題被解決,確保沒有引入新問題。
被影響的同事可以及時(shí)了解他受到的影響。
團(tuán)隊(duì)Leader或者產(chǎn)品架構(gòu)師如果覺得PR邀請(qǐng)的審核者不足或者過多,必須調(diào)整為合適的人員,其它同事可以在評(píng)論中建議。
三、收獲
我們團(tuán)隊(duì)實(shí)施Code Review收獲不少,總結(jié)出來大概有以下幾點(diǎn):
1、短期內(nèi)迅速提高了代碼質(zhì)量。
原因有幾個(gè),大家知道自己的代碼會(huì)被人審核之后寫得會(huì)比較認(rèn)真。
理論上代碼質(zhì)量是由整個(gè)團(tuán)隊(duì)內(nèi)***秀的那個(gè)人決定的。
大家也能在Review的過程中學(xué)習(xí)到其它同事優(yōu)秀的編碼。
2、Bug數(shù)量迅速減少。
但是這個(gè)我們沒有數(shù)據(jù)統(tǒng)計(jì)比較,比較遺憾。
我和QA聊過,他給我的數(shù)據(jù)是在我們的一個(gè)新項(xiàng)目每2周一次的大發(fā)布,平均只會(huì)發(fā)現(xiàn)1~2個(gè)Bug。
這點(diǎn)提高了整個(gè)團(tuán)隊(duì)的幸福感,大家不用經(jīng)常被火燒眉毛。
3、團(tuán)隊(duì)成員對(duì)項(xiàng)目的熟悉程度會(huì)比較均衡。
新同事通過參與Code Review能很快熟悉團(tuán)隊(duì)的規(guī)范。
代碼不會(huì)只有個(gè)別人了解、熟悉,Bug誰都能改,新功能誰都能做。
對(duì)公司來說避免了人員的風(fēng)險(xiǎn),對(duì)個(gè)人來說比較輕松(誰都能來幫你),可以選自己喜歡的任務(wù)做。
4、改善團(tuán)隊(duì)的氛圍
Review的過程中會(huì)需要非常多的溝通,多溝通能拉近團(tuán)隊(duì)成員的距離。
并且無論級(jí)別高低,大家的代碼都是要經(jīng)過Review的,可以在團(tuán)隊(duì)內(nèi)營(yíng)造一個(gè)平等的氛圍。
每個(gè)成員都可以審查別人的代碼,這很容易激發(fā)他們的積極性。
亮一下我們的數(shù)據(jù):
我們從2014年1月17日開始***個(gè)PR的提交,到2016年7月5日一共發(fā)出了6944個(gè)PR,其中6171個(gè)通過,739個(gè)拒絕。日均11.85個(gè)PR,最多的一天提了55個(gè)PR。
這些PR一共產(chǎn)生了30040個(gè)評(píng)論,平均每個(gè)PR有4.32個(gè)評(píng)論,最多的一個(gè)PR有239個(gè)評(píng)論。
參與上述PR評(píng)論的同事一共有53位,平均每位同事發(fā)出了539個(gè)評(píng)論,最多的用戶發(fā)出了5311個(gè)評(píng)論,最少的發(fā)了1個(gè)(剛推行Code Review就離職的同事)。
需要說明一下,只有簡(jiǎn)單的問題會(huì)通過評(píng)論來提出。比較復(fù)雜的,比如涉及到架構(gòu)、安全等方面的問題,其實(shí)都會(huì)面對(duì)面的溝通,因?yàn)檫@樣效率更高。
四、總結(jié)
雖然有合適的工具支持會(huì)更容易實(shí)施Code Review,但它本身并不特別依賴具體的工具,所以前文并沒有具體指明我們用了什么工具,除了Git。
原因是基于分支的PR流程依賴于大量創(chuàng)建分支,而Git創(chuàng)建一個(gè)分支非常的簡(jiǎn)單,所以PR模式+Git是一個(gè)很好的搭配。
我們?cè)谇袚Q到Git之前,也做Code Review,采用的是提交代碼以后把commit的Id發(fā)給相關(guān)同事來審查的流程。
審核通過以后會(huì)在缺陷跟蹤管理系統(tǒng)里面評(píng)論,QA同事沒見到審核通過的評(píng)論就認(rèn)為任務(wù)沒有完成,拒絕進(jìn)行測(cè)試。
雖然沒有現(xiàn)在這樣直接方便,但是也還是做起來了。
PR審核的過程中,新加入的團(tuán)隊(duì)成員常見的問題是不符合代碼規(guī)范之類的,其實(shí)是可以通過源代碼檢查工具來解決的,這部分我們一直在計(jì)劃中(( ╯□╰ )),并沒有開始實(shí)施。
網(wǎng)頁(yè)名稱:我們是怎么做CodeReview的
網(wǎng)站URL:http://fisionsoft.com.cn/article/dpdgcgi.html


咨詢
建站咨詢
