前幾天看了《Code Review 程式設計師的寄望與哀傷》，想到我們團隊開展Code Review也有2年了，結果還算比較滿意，有些經驗應該可以和大家一起分享、探討。 我們為什麼要推行Code Review呢？我們當時面臨著程式碼混亂、Bug頻出的狀況。 當時我覺得要有所改變，希望能提高產品的程式碼質量，改善開發團隊面臨的困境。並且我個人在開發上有很多經驗，也希望這些知識能夠在團隊內傳播。 各種考慮後，我們最後認為推行Code Review能改善或解決我們面臨的很多問題。 這篇文章的目的不是告訴大家怎麼在一個團隊內推行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。 評論可以被所有有許可權檢視倉庫的人看到，每個人都可以回覆任何人的評論，有點像論壇裡某個帖子的討論。 這種模式是事後稽核，也就是程式碼已經提交到了中心倉庫，Review過程中頻繁的改動會造成歷史簽入記錄的混亂。 當然Git可以採用更改歷史記錄來解決這個問題，由於容易誤操作，我們一般只在基礎類庫這類要求比較嚴格的專案上實施。

我們所瞭解到的支援PR模式的軟體都採用Git作為原始碼版本控制工具，所以我們的原始碼版本控制工具也遷移到了Git。 由於Git太靈活了，因此誕生了很多的Git流程，用來規範Git的使用。 常見的有集中式工作流、功能分支工作流、Gitflow工作流、Forking工作流、Github工作流。 我們對Git Flow做了些調整，調整後的流程被命名為Baza Flow，定義見後文。 根據Baza Flow，我們大部分倉庫只定義了2個主幹分支，master和develop。(例外，我們有一個倉庫有3個開發小組同時進行開發，定義了4個主幹分支，目前還比較順暢，再多估計主幹分支之間的合併就比較繁瑣了。) master對應生產環境程式碼，所有面向生產環境的釋出來源都是master分支的程式碼。develop則對應本地測試環境的程式碼。 絕大多數情況下，QA（測試）只測試develop分支和master分支的程式碼。

由於開發人員都在一個團隊內，所以我們沒有采用基於倉庫的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上發給程式碼稽核者，以此通知對方及時進行稽核。

二、執行

我們在團隊內部提倡質量優先，開發團隊不能為了進度犧牲質量，並在團隊內部達成了共識。 所以，無論進度有多麼緊迫，Code Review的過程都一定會做。 所有的問題一定會被提出，只是會根據進度的緊迫程度，以及問題的大小，改動成本，決定問題是現在解決，還是加一個TODO，並記錄在缺陷跟蹤管理系統內，以防日後遺忘。 多數情況下，我們都會要求立即解決，哪怕因此造成了釋出的推遲。 我們深知，其實多數情況下，現在不解決，日後不知道猴年馬月才能解決。

我們在團隊內推行Code Review的過程中沒有遇到太多阻力。 原因大概有兩點，首先管理層方面瞭解之前遇到的各種問題，也迫切希望能有所改善，所以從一開始就是支援的態度。 其次，絕大部分開發人員覺得在這個過程中能自己能學習到東西，並沒有抵觸，遇到很好的意見時大家都還是很高興的。 最後，慢慢的形成了一種氛圍，整個團隊都會自覺的維護它。 附一張我們稽核的對話圖，這位童鞋嘗試對系統內部散落各地發業務郵件的程式碼做一個整理，用一套模式來處理，調整了3版才定調，然後修改了很多細節才通過了合併，前後大概用一個多星期時間：