咱們是怎麼作Code Review的

前幾天看了《Code Review 程序員的寄望與哀傷》，想到咱們團隊開展Code Review也有2年了，結果還算比較滿意，有些經驗應該能夠和你們一塊兒分享、探討。
咱們爲何要推行Code Review呢？咱們當時面臨着代碼混亂、Bug頻出的情況。
當時我以爲要有所改變，但願能提升產品的代碼質量，改善開發團隊面臨的困境。而且我我的在開發上有不少經驗，也但願這些知識可以在團隊內傳播。
各類考慮後，咱們最後認爲推行Code Review能改善或解決咱們面臨的不少問題。

這篇文章的目的不是告訴你們怎麼在一個團隊內推行Code Review，首先由於我我的僅在一家公司內推行過，並無不少經驗。
其次每家公司、每一個團隊的狀況都不太同樣，應該根據公司或團隊的實際狀況選擇恰當的方案，並根據成員的反饋來及時調整，推進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，這是強制的。

 

Baza Flow

當前版本 V0.9

Baza Flow 由 Git Flow 演化而來，Git Flow的開發模式以下圖所示：

因爲咱們的託管軟件對於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句話以內）來描述： 

你改動了什麼，解決了什麼問題，須要代碼審查的人留意那些影響比較大的改動。
特別須要留意，若是對基礎、公共的組件進行了改動，必定要另起一行特別說明。

 審覈人員邀請原則： 

1. 在建立PR時，Reviewers（審覈人）一欄裏主要填寫「必需審覈人」。只有這些人審覈都經過，才容許合併。
2. 除了「必需審覈人」外，還有一些其它審覈人，咱們能夠在Description裏作爲「邀請審覈嘉賓」@進來。
3. 主幹分支間的合併，如Develop => Master，或Master => Develop等，則須要把整個團隊（開發+QA）都列爲「必需審覈人」。 

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

• 團隊Leader
• 前端架構師（若是有前端代碼改動） （能夠受權）
• 後端架構師（若是有後端代碼改動） （能夠受權）
• 產品架構師
• 對此PR解決的問題比較熟悉的（以前一直負責這部分業務的同事）
• 此PR解決的問題對他影響比較大（好比認領的任務依賴此PR的同事）

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

須要知悉此處代碼改動的人但又沒必要非要其審覈經過的同事
能夠從這個PR中學習的同事 

能夠受權指的是，根據約定，Bug修復之類的改動，或者影響較小的改動，前端架構師和後端架構師能夠受權團隊內的某個資深開發人員，由這個資深開發人員表明他們進行審覈。
主幹分支之間的合併，大型Feature的合併，前端架構師和後端架構師須要參與。 

上述審覈人關注的視角不太同樣：
團隊Leader關注你是否完成了任務，先後端架構師關注是否符合公司統一的架構、風格、質量，產品架構師從整個產品層面來關注這個PR。
熟悉此問題的同事能夠更好的保證問題被解決，確保沒有引入新問題。
被影響的同事能夠及時瞭解他受到的影響。 

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

3、收穫

咱們團隊實施Code Review收穫很多，總結出來大概有如下幾點：

一、短時間內迅速提升了代碼質量。
     緣由有幾個，你們知道本身的代碼會被人審覈以後寫得會比較認真。
     理論上代碼質量是由整個團隊內最優秀的那我的決定的。
     你們也能在Review的過程當中學習到其它同事優秀的編碼。

二、Bug數量迅速減小。
    可是這個咱們沒有數據統計比較，比較遺憾。
    我和QA聊過，他給個人數據是在咱們的一個新項目每2週一次的大發布，平均只會發現1~2個Bug。
    這點提升了整個團隊的幸福感，你們不用常常被迫不及待。

三、團隊成員對項目的熟悉程度會比較均衡。
    新同事經過參與Code Review能很快熟悉團隊的規範。
    代碼不會只有個別人瞭解、熟悉，Bug誰都能改，新功能誰都能作。
    對公司來講避免了人員的風險，對我的來講比較輕鬆（誰都能來幫你），能夠選本身喜歡的任務作。

四、改善團隊的氛圍
    Review的過程當中會須要很是多的溝通，多溝通能拉近團隊成員的距離。
    而且不管級別高低，你們的代碼都是要通過Review的，能夠在團隊內營造一個平等的氛圍。
    每一個成員均可以審查別人的代碼，這很容易激發他們的積極性。

亮一下咱們的數據:

咱們從2014年1月17日開始第一個PR的提交，到2016年7月5日一共發出了6944個PR，其中6171個經過，739個拒絕。日均11.85個PR，最多的一天提了55個PR。
這些PR一共產生了30040個評論，平均每一個PR有4.32個評論，最多的一個PR有239個評論。
參與上述PR評論的同事一共有53位，平均每位同事發出了539個評論，最多的用戶發出了5311個評論，最少的發了1個（剛推行Code Review就離職的同事）。
須要說明一下，只有簡單的問題會經過評論來提出。比較複雜的，好比涉及到架構、安全等方面的問題，其實都會面對面的溝通，由於這樣效率更高。

4、總結

雖然有合適的工具支持會更容易實施Code Review，但它自己並不特別依賴具體的工具，因此前文並無具體指明咱們用了什麼工具，除了Git。
緣由是基於分支的PR流程依賴於大量建立分支，而Git建立一個分支很是的簡單，因此PR模式+Git是一個很好的搭配。
咱們在切換到Git以前，也作Code Review，採用的是提交代碼之後把commit的Id發給相關同事來審查的流程。
審覈經過之後會在缺陷跟蹤管理系統裏面評論，QA同事沒見到審覈經過的評論就認爲任務沒有完成，拒絕進行測試。
雖然沒有如今這樣直接方便，可是也仍是作起來了。

PR審覈的過程當中，新加入的團隊成員常見的問題是不符合代碼規範之類的，實際上是能夠經過源代碼檢查工具來解決的，這部分咱們一直在計劃中（( ╯□╰ )），並無開始實施。

轉自：https://www.cnblogs.com/wenhx/p/How-We-Code-Review.html