從零開始 Code Review，兩年實戰經驗分享！

程序員的成長之路

互聯網/程序員/成長/職場

閱讀本文大概須要 7.4 分鐘。javascript

重要通知：文末有周末中獎的名單，中獎的小夥伴儘快添加我微信。css

前幾天看了《Code Review 程序員的寄望與哀傷》，想到咱們團隊開展 Code Review 也有 2 年了，結果還算比較滿意，有些經驗應該能夠和你們一塊兒分享、探討。
html

咱們爲何要推行 Code Review 呢？咱們當時面臨着代碼混亂、 Bug 頻出的情況。前端

當時我以爲要有所改變，但願能提升產品的代碼質量，改善開發團隊面臨的困境。而且我我的在開發上有不少經驗，也但願這些知識可以在團隊內傳播。java

各類考慮後，咱們最後認爲推行 Code Review 能改善或解決咱們面臨的不少問題。程序員

這篇文章的目的不是告訴你們怎麼在一個團隊內推行 Code Review，首先由於我我的僅在一家公司內推行過，並無不少經驗。web

其次每家公司、每一個團隊的狀況都不太同樣，應該根據公司或團隊的實際狀況選擇恰當的方案，並根據成員的反饋來及時調整，推進 Code Review 的實施。面試

因此，本文是介紹咱們公司是如何實施 Code Review 的，咱們是如何解決咱們遇到的問題的，但願咱們的經驗能給你們帶來些幫助。算法

行文倉促，若有遺漏或錯誤，歡迎指正。數據庫

▌1、流程和規則

通過簡單的對比、試用，咱們最後採用了 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，這是強制的。

因爲咱們的託管軟件對於 Pull Request 的限制，咱們對 Git Flow 作了改動，改動的地方有：

一、每個大功能咱們會建立一個單獨的 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 後建立新任務分支的時候務必知會一下相關配合同事（好比前端的同事），讓他們在新的分支上繼續開發。

二、對於小功能，預計在 0.5-1 個（不超過 2 個）工做日工做量的開發任務，直接基於 develop 分支建立特性分支便可。

三、在各個分支遇到的 bug，請基於該分支建立一個 Bug 分支。

若是在缺陷跟蹤管理系統上有對應的項，命名請使用缺陷跟蹤管理系統的 ID，好比 BAZABUG-1354 好比這個 Bug 的分支命名就是 bugfix/BAZABUG-1354。

若是在缺陷跟蹤管理系統上沒有對應的項，命名請簡短的說明修改內容，好比「JX 9df2b01 引用 bootstrap css 虛擬路徑重寫，避免出現字體沒法找到的問題」，分支命名能夠是 bugfix/miss-font。

完成修改之後提交併推送到中心倉庫而後當即向上遊分支提交 pull request。

四、發起 pull request 之後，請將 pull request 的連接在 IM 上發給代碼審覈者，以此通知對方及時進行審覈。

▌2、執行

咱們在團隊內部提倡質量優先，開發團隊不能爲了進度犧牲質量，並在團隊內部達成了共識。

因此，不管進度有多麼緊迫，Code Review 的過程都必定會作。

全部的問題必定會被提出，只是會根據進度的緊迫程度，以及問題的大小，改動成本，決定問題是如今解決，仍是加一個 TODO，並記錄在缺陷跟蹤管理系統內，以防往後遺忘。

多數狀況下，咱們都會要求當即解決，哪怕所以形成了發佈的推遲。

咱們深知，其實多數狀況下，如今不解決，往後不知道猴年馬月才能解決。

咱們在團隊內推行 Code Review 的過程當中沒有遇到太多阻力。

緣由大概有兩點，首先管理層方面瞭解以前遇到的各類問題，也迫切但願能有所改善，因此從一開始就是支持的態度。

其次，絕大部分開發人員以爲在這個過程當中能本身能學習到東西，並無抵觸，遇到很好的意見時你們都仍是很高興的。

最後，慢慢的造成了一種氛圍，整個團隊都會自覺的維護它。

附一張咱們審覈的對話圖，這位童鞋嘗試對系統內部散落各地發業務郵件的代碼作一個整理，用一套模式來處理，調整了 3 版才定調，而後修改了不少細節才經過了合併，先後大概用一個多星期時間：

表面上看來 Code Review 會延緩項目的進度，可是在咱們 2 年多的執行過程當中，大多數時候沒感受到有延緩。

緣由是，雖然代碼合併的週期變長了，可是因爲代碼質量提升了，致使 Bug 變少了，因爲 Bug 引發的返工問題也變少了，所以總體的進度其實並無延緩。

我我的認爲對一個成熟的團隊其實作 Code Review 反而會加快總體的項目進度，可是手頭上沒有統計數據支撐個人觀點。（對於軟件開發的度量，歡迎有心得的同窗告知我）

咱們每一個分支有權限合併的人都不止一個，這樣能夠保證有人請假不在的時候，代碼仍然能夠被其餘同事審覈經過以後合併。

半年前，咱們團隊加入了不少新成員，剛加入的新同事對規範、項目、產品的熟悉程度都不高，致使了有一段時間，咱們遇到了 PR 審覈週期變長的問題。

加上以前遇到的一些問題，咱們總結了一個說明，目的是減輕 Code Review 對開發人員工做的負擔，加快 PR 審覈經過的過程。

說明以下：

Pull Request 的說明

任務完成才能提交 PR。

PR 應該在一個工做日內被合併或者被拒絕。

PR 在有嚴重問題（包括但不限於架構問題、安全問題、設計問題），太多問題，或者任務無效的狀況下會被拒絕。

嚴禁一個PR裏面有多個任務，除非它們是緊密關聯的。

PR 提交以後只容許針對 Review 發現問題再次提交代碼，除非有充足的理由，嚴禁在同一個 PR 中再次提交其它任務的代碼。

提交PR時候有一個描述框，內容會自動根據 Commit 的 message 合併而成。

切記，若是一次提交的內容包含不少 Commit，請不要使用自動生成的描述。

請用簡短可是足夠說明問題的語言（理想是控制在3句話以內）來描述：

你改動了什麼，解決了什麼問題，須要代碼審查的人留意那些影響比較大的改動。

特別須要留意，若是對基礎、公共的組件進行了改動，必定要另起一行特別說明。

審覈人員邀請原則：

在建立 PR 時，Reviewers（審覈人）一欄裏主要填寫「必需審覈人」。只有這些人審覈都經過，才容許合併。

除了「必需審覈人」外，還有一些其它審覈人，咱們能夠在 Description 裏作爲「邀請審覈嘉賓」@進來。

主幹分支間的合併，如 Develop => Master，或Master => Develop 等，則須要把整個團隊（開發+QA）都列爲「必需審覈人」。

必須審覈人的列表由團隊決定，可能包括如下人選：

團隊 Leader

前端架構師（若是有前端代碼改動）（能夠受權）

後端架構師（若是有後端代碼改動）（能夠受權）

產品架構師

對此 PR 解決的問題比較熟悉的（以前一直負責這部分業務的同事）

此PR解決的問題對他影響比較大（好比認領的任務依賴此PR的同事）

其它審覈人，包括但不限於：

須要知悉此處代碼改動的人但又沒必要非要其審覈經過的同事。

能夠從這個 PR 中學習的同事。

能夠受權指的是，根據約定，Bug 修復之類的改動，或者影響較小的改動，前端架構師和後端架構師能夠受權團隊內的某個資深開發人員，由這個資深開發人員表明他們進行審覈。

主幹分支之間的合併，大型 Feature 的合併，前端架構師和後端架構師須要參與。

上述審覈人關注的視角不太同樣：

團隊 Leader 關注你是否完成了任務，先後端架構師關注是否符合公司統一的架構、風格、質量，產品架構師從整個產品層面來關注這個 PR。

熟悉此問題的同事能夠更好的保證問題被解決，確保沒有引入新問題。

被影響的同事能夠及時瞭解他受到的影響。

團隊 Leader 或者產品架構師若是以爲 PR 邀請的審覈者不足或者過多，必須調整爲合適的人員，其它同事能夠在評論中建議。