【轉】code review的方式
前幾天看了《Code Review 程式設計師的寄望與哀傷》,想到我們團隊開展Code Review也有2年了,結果還算比較滿意,有些經驗應該可以和大家一起分享、探討。我們為什麼要推行Code Review呢?我們當時面臨著程式碼混亂、Bug頻出的狀況。當時我覺得要有所改變,希望能提高產品的程式碼質量,改善開發團隊面臨的困境。並且我個人在開發上有很多經驗,也希望這些知識能夠在團隊內傳播。各種考慮後,我們最後認為推行Code Review能改善或解決我們面臨的很多問題。這篇文章的目的不是告訴大家怎麼在一個團隊內推行Code Review,首先因為我個人僅在一家公司內推行過,並沒有很多經驗。其次每家公司、每個團隊的情況都不太一樣,應該根據公司或團隊的實際情況選擇恰當的方案,並根據成員的反饋來及時調整,推動Code Review的實施。
一、流程和規則
經過簡單的對比、試用,我們最後採用了Git Flow+Pull Request(PR)模式來做Code Review。(PR模式詳情可參見 Git工作流指南:Pull Request工作流)
Pull Request(PR)簡單的說就是你沒有許可權往一個特定的倉庫或分支提交程式碼,你請求有許可權的人把你提交的程式碼從你的倉庫或分支合併到指定的倉庫或分支。由於PR需要有許可權的人確認,所以非常適合在這個過程中做Code Review,是否接受或者拒絕就取決於Code Review的結果。在支援PR模式的軟體裡,每一個PR都有一個新增程式碼的對比(diff)介面。程式碼稽核者可以線上瀏覽請求合併的新增程式碼,並針對有疑問的程式碼行新增評論,通過這種方式來實現Code Review。評論可以被所有有許可權檢視倉庫的人看到,每個人都可以回覆任何人的評論,有點像論壇裡某個帖子的討論。
我們所瞭解到的支援PR模式的軟體都採用Git作為原始碼版本控制工具,所以我們的原始碼版本控制工具也遷移到了Git。由於Git太靈活了,因此誕生了很多的Git流程,用來規範Git的使用。根據Baza Flow,我們大部分倉庫只定義了2個主幹分支,master和develop。(例外,我們有一個倉庫有3個開發小組同時進行開發,定義了4個主幹分支,目前還比較順暢,再多估計主幹分支之間的合併就比較繁瑣了。)
由於開發人員都在一個團隊內,所以我們沒有采用基於倉庫的PR,採用的是基於分支的PR。我們對主幹分支的操作許可權做了限制,只有特定的人才能操作,develop分支是專案開發Leader和架構師,master分支是QA。有許可權往主幹分支合併的成員會按照約定的規則來執行合併,不會合並沒有完成稽核的PR。上面這點其實蠻重要的,所以我們會對有許可權合併的人有特別的約定,在什麼情況下才能合併程式碼。(見後文PR的說明)PR的發起人要主動的推動PR的稽核,Leader也會密切關注PR稽核的進度,在需要的時候及時介入。
我們配置了CI伺服器(什麼是CI)只編譯特定的分支,通常是develop和master分支。所有的程式碼合併到了主幹分支之後,都會自動觸發編譯和本地測試環境的釋出,QA無需依賴開發人員編譯的程式碼來測試,也無需自己手工操作這些,保證了開發人員和測試人員的相互獨立。我們本地測試環境的釋出包含了資料庫和站點的釋出,全自動的,釋出完成以後就是一個可用的產品,有時間這部分也可以分享一下。
我們還使用了Scrum裡面一個很重要的概念:完成定義。就是我們規定了我們一個任務的完成被定義為:程式碼編寫完成,經過自測,提交的PR經過稽核並且合併到主幹分支。也就是說,所有的程式碼被合併到了主幹分支之後任務才算是完成,而被合併到主幹分支必須要經過Code Review,這是強制的。
Baza Flow
當前版本 V0.9
Baza Flow 由 Git Flow 演化而來,Git Flow的開發模式如下圖所示:
由於我們的託管軟體對於Pull Request的限制,我們對Git Flow做了改動,改動的地方有:1、每一個大功能我們會建立一個單獨的feature分支,專案開發人員基於這個單獨的feature分支建立自己的任務分支。 比如,對於CS 2專案來說,啟動的時候分支的建立是:master -> develop -> feature/v2。 開發人員應該基於這個大特性分支feature/v2來建立自己的任務分支,比如建立XXXX,可以用一個單獨的分支feature/v2-xxxx。 完成這個任務以後,立即向上遊分支(feature/v2)提交pull request。然後從feature/v2-xxxx 建立自己的下一個任務分支,比如YYYY編輯 feature/v2-yyyy。 請注意,合併到上游分支的功能必須相對獨立而且是可用的,分支任務工作量0.5-1個工作日,不宜超過2個工作日,超過2個工作日不向上游合併,需要向團隊解釋。 程式碼經過Review以後,可能會進行必要的修改,修改在原分支修改,修改完畢程式碼合併進上游分支,原分支會定期刪除。 專案組成員在收到合併成功的通知後,請自行從上游大特性分支向下合併到自己當前的開發分支。 提交pull request後建立新任務分支的時候務必知會一下相關配合同事(比如前端的同事),讓他們在新的分支上繼續開發。
2、對於小功能,預計在0.5-1個(不超過2個)工作日工作量的開發任務,直接基於develop分支建立特性分支即可。
3、在各個分支遇到的bug,請基於該分支建立一個Bug分支。 如果在缺陷跟蹤管理系統上有對應的項,命名請使用缺陷跟蹤管理系統的ID,比如BAZABUG-1354 比如這個Bug的分支命名就是bugfix/BAZABUG-1354。 如果在缺陷跟蹤管理系統上沒有對應的項,命名請簡短的說明修改內容,比如“JX 9df2b01 引用bootstrap css虛擬路徑重寫,避免出現字型無法找到的問題”,分支命名可以是bugfix/miss-font。 完成修改以後提交併推送到中心倉庫然後立即向上遊分支提交pull request。4、發起pull request以後,請將pull request的連結在IM上發給程式碼稽核者,以此通知對方及時進行稽核。