代碼評審--流程、工具和最佳實踐

為什麼執行代碼評審

1.      代碼評審的最大好處是純社會性的，當你知道你的每一行代碼都有另外一個人看，自然而然會更加賣力的表現，拿出最好的狀态編碼，因為每個人都有虛榮心嘛；

2.      代碼評審可以及時發現一些容易發現的BUG，而不必将發現BUG的時間點推遲到測試階段；

3.      代碼評審可以保證至少有兩個人都了解任何一份代碼。當出現員工休假，離職等情況的時候，至少保證團隊的代碼不會陷入無人了解或者無人處理的狀況。

流程篇

代碼評審的流程有以下兩種：

1.      送出前評審(pre-push code review)

2.      送出後評審(post-push code review)

送出前評審的流程如下：

1.      程式員在試圖送出代碼變更到代碼庫之前，先送出變更申請，變更申請包含了這次變更的内容，評審人；

2.      評審人檢視變更内容，評估變更，與變更申請人溝通，評估是否通過變更；

3.      如果評審人通過變更，則變更申請人才可以送出代碼到代碼庫；

4.      如果評審人不通過變更，則變更申請人需要根據讨論結果或評審建議做出修改，直到與評審人達成一緻，通過評審，才可以送出代碼到代碼庫；

送出後評審的流程如下：

1.      程式員送出變更代碼到代碼庫；

2.      評審人審查這次變更的内容，如果評審通過，則标記此次的變更已審查；

3.      如果評審人有疑義，則與變更人溝通，變更人根據讨論結果或評審意見做出修改，知道與評審人達成一緻，通過評審。

pre-push code review vs post-push codereview

Google，Facebook都嚴格執行pre-pushcode review。送出前評審對比送出後評審有諸多好處：

1.      程式員會更積極的将變更的代碼組織的更好，更子產品化，更容易閱讀；

2.      評審人有機會在代碼送出之前發現問題，或給出更好的建議，對應的程式員對這樣的回報更容易接受；

3.      評審人給出建議或意見之後，相比送出後評審，程式員會更加積極的最回報做出響應；

4.      評審人會更加認真的對變更進行評審，并且發現問題後會更加積極的參與讨論，對發起變更的程式員提供支援；

5.      在真正送出變更前發現問題并予以解決顯然比送出後再進行評審，然後送出修改更新檔更好；

post-push code review的好處：

1.      普遍認為相比pre-push code review，post-push code review幾乎沒有任何好處！

2.      但是post-push code review 更加容易實施，過程對現有的組織架構和流程沒有完全的颠覆，對團隊成員的要求沒有那麼高；

3.      相比pre-push code review，post-push code review不需要對修改代碼&送出變更這個過程中斷，不需要等待評審的時間；

4.      可以作為組織向pre-push code review過程實施的過渡訓練；

工具篇

Facebook 的代碼評審工具 Phabricator簡介

使用phabricator的理由，一條就打動我了：phabricator被認為facebook成功的11個重要因素或者工具之一，facebook的工程師對這個工具贊譽有加。

phabricator已經開源，在github可以下載下傳。

phabricator對pre-push code review和 post-push code review 過程都支援，分别是differential和 audit。

不必因為phabricator是facebook的産品，而擔心它是重量級的工具。事實上，它完全支援小團隊開發，甚至兩個人的團隊都可以使用進行代碼評審。

關于Phabricator的安裝和配置就不介紹了，參考

http://www.phabricator.com/docs/phabricator/index.html

中間有些配置選項需要注意，尤其在配置arc工具以及配置repository的時候，本地化以及svn的根目錄等相關細節。

如果安裝配置和使用過程中有問題，歡迎留言或者微網誌私信讨論。

最佳實踐篇：

Google工程師的重要建議：

（此處參考：http://www.aqee.net/things-everyone-should-do-code-review/）

最重要的一個原則：代碼審查用意是在代碼送出前找到其中的問題——你要發現是它的正确。在代碼審查中最常犯的錯誤——幾乎每個新手都會犯的錯誤——是，審查者根據自己的程式設計習慣來評判别人的代碼。

對于一個問題，通常我們能找出十幾種方法去解決。對于一種解決方案，我們能有百萬種編碼方案來實作它。作為一個審查者，你的任務不是來確定被審查的代碼都采用的是你的編碼風格——因為它不可能跟你寫的一樣。作為一段代碼的審查者的任務是確定由作者自己寫出的代碼是正确的。一旦這個原則被打破，你最終将會倍感折磨，深受挫折——這可不是我們想要的結果。

ps: 我見過很多程式員都有心理潔癖，總是要求其他人按照标準的格式，标準的思路去設計或者編碼，要知道，program是創造性的工作，程式員有獨立的個性，思考方法，表現方式都是很正常的。

另外在IBM developerworks 上有一篇介紹代碼評審的最佳實踐的文章寫得相當好，參考：

http://www.ibm.com/developerworks/rational/library/11-proven-practices-for-peer-review/

其中大部分最佳實踐都可以通過自己實踐，慢慢摸索，尋找最适合團隊執行的模式和規則。我認為最重要的有兩條，一是創造良好的評審氛圍。這個與上面提到的代碼評審時發現正确而不是發現錯誤是一樣的道理。另外一條是采用輕量級的工具，的确如此，之前經曆的團隊評審代碼基本都沒有采用工具，直接檢視svn代碼log，不僅效率不高，對bug發現率也有很大影響。工具采用要輕量級。

SVN Commit 篇

這一篇是筆者這幾年經曆過項目中存在的問題的思考

1.      Svn 送出的日志要寫清楚，盡量準确的描述此次變更的内容，比如：修改了某某某某bug，增加了什麼具體功能。很多程式員包括老程式員對svn日志很無所謂，寫得log内容諸如“做一些修改”，“update”等毫無意義的log。這樣的log不僅對代碼評審毫無意義，在修改bug或者版本復原時更是令人惱火。

2.      Svn 送出的次數和送出的内容控制。有兩種情況，一種是無限制的随意送出svn，随意的幾行變更就迫不及待的送出；另外一種是好多天都不送出，積累了非常多代碼之後，進行一次性更新。很多程式員對送出太随意，最常見的情況是釋出版本前，送出未經測試的代碼，然後發現問題，不斷的送出修改，每一個修改都隻有兩三行。嚴重破壞svn版本log的可讀性。比較好的做法是，對一個子產品，完成修改之後，在本地執行單元測試之後，統一送出，送出的代碼不宜太少，也不宜太多。如果送出代碼太多，對代碼評審和版本復原造成不便。這種情況，可以考慮細分成一些相關的變更，分多次送出。