讓 Code Review成爲一種習慣

1. 開篇

5月份的時候忽然接到 code.oa.com【騰訊內部的一個代碼管理平臺】 的 summer 的通知， 說廣點通的codereview 參與度在公司各部門中表現出色，而咱們小組（廣點通廣告定向小組）的 codereview 綜合表如今全公司的小組中排名第一。這讓我有點意外，有點無意插柳的感受。 summer 但願我能寫寫咱們小組作 code review 的一些心得體驗， 回想我學習使用 codereview 的經歷， 我最早想到的一句話是我幾年前看到的關於 codereview 的一句評述：

The biggest thing that makes Google’s code so good is simple: code review.That’s not specific to Google – it’s widely recognized as a good idea, and a lot of people do it. But I’ve never seen another large company where it was such a universal. At Google, no code, for any product, for any project, gets checked in until it gets a positive review.

這是一段讓我記憶深入的話， 一個Google 工程師寫的， 我摘錄下來作做爲文章的開始，表達我在騰訊這些年和一些出色的工程師合做的過程當中一塊兒學習和實踐 CodeReview 以後， 對CodeReview 文化的一種由衷的認同和尊敬。

2. Rietveld

我 2008 年進入騰訊， 主要呆在 R2參與soso 的開發， 從 2008 ~ 2010 年我都很不屑於 R2 的研發質量管理，以爲作得實在很糟糕：沒有統一的編碼規範、沒有unittest，甚至經歷過有些人不會用 svn, 不太懂代碼的版本控制， 固然我本身也不怎麼懂開發流程中的質量管理， 那時候連 codereview 是什麼都沒據說過。2010年初， 公司開始在搜索上加大投入， 招聘了一些牛人，尤爲是 Google 的一些工程師， 而這些人也把 Google 的一些開發文化帶入了 R2。雖然 soso 最終的命運並不如人所願， 可是在 2010~2013年間， 許多soso 的工程師是學習並經歷了什麼是嚴謹而強大的開發流程，也見過咱們曾經山寨的 Google 的技術， 以及山寨的 Google 的開發流程。 而至今爲止，咱們和不少同事回憶起來，咱們常調侃說咱們是見過豬跑也吃過豬肉的。

2010年4月從Google 來到咱們部門的一位牛人是 yiwang, 作機器學習的博士， 作了一次 LDA topic modeling 的報告， 使人印象深入， 接觸了幾回以後，我主動找了 yiwang 一塊兒合做，學習 LDA 並參與並行 topic modeling 的開發。yiwang 讓我作的第一件事就是研究一下 Google 開源的 Rietveld code review 系統， 並找一臺機器把 Rietveld 搭建起來。因而一頓折騰以後， 咱們搭建了公司的第一套嚴格意義上的 codereview 系統， 固然那個時候也只是供咱們小組內本身使用。 搭建好 Rietveld 以後， 咱們作的第二件事就是把 code style 都統一到 Google 發佈的 C++ coding style。 最先在這套系統上進行 codereview 的就是4個工程師 yiwang, leostarzhou, charlieyan, 和我。

全部和 yiwang 合做過的人在 codereview 中都苦不堪言， 他是近乎苛刻的嚴格執行 Google C++ coding style, 咱們提交的的 code 中多一個空格，多一個空行；或者是註釋符號 「//」 後面少一個空格， 全都被打回來修改；更別說變量、函數名命名不符合規範，隨意使用變量縮寫這些大的禁區了。對於咱們最初的代碼， yiwang 挨個細緻的指出咱們違反規範的地方， 在 comment 中貼上Google C++ 規範的具體要求的 URL anchor， 以說明咱們爲什麼違反了規範。 一開始我是有點不覺得然的， 來回幾回以後我本身也很差意思了， 把 Google coding style 從頭至尾仔仔細細的看上3遍， 認真的記住一些細節， 因而很快就開始上手了。

天然， 後續咱們合做的工程師逐漸多了起來， 對於 codereview 的必要性，許多人是有質疑和挑戰的， 因而在和yiwang合做的幾年裏， 我反覆聽他說了三個故事。

第一個是關於他本身的，他從小開始 coding, 高中的時候已經拿到工程師的認證，進入 Google 以前 coding 無數， 進入 Google 以後寫的第一個100行的程序， 被貼的 comment 超過了100行。 因此不要抱怨咱們的標準嚴格， 嚴格的標準才能產出漂亮的代碼， 咱們今天在騰訊作的 codereview 真是不算啥。

第二個是關於Google 內部 codereview 的爭議的， 聽說 Google 內部創建 codereview 制度的時候，也是不少工程師不樂意， 尤爲是年長的工程師， 寫了多少年代碼了， 被一羣年輕的小屁孩在代碼上指指點點，實在是受不了。 最後說是 Google 的一位極有地位的人物出來講話了：咱們要想把工程質量控制好， 不管是資歷深淺，都必定要遵照共同的代碼規範，因而 codereview 制度得以創建。

第三個是解釋爲什麼要用 Google C++ coding style 的， Google 的代碼規範是一羣頂尖的 coder 制定的， 變量名用小寫， 函數名用駝峯式， 那都是有嚴格的視覺美學講究的，讓人看了很舒服的，甚至是作過視覺對照研究的。雖然我對此存疑, 我仍是認同 Google C++ coding style 是我見過的最好的 coding style。 而咱們騰訊的一位 HR 在和我閒聊的時候也表示過一樣的觀點 :-)。

Coding style 的規則制定不少都是相對的， 而Google style作得 NB 的地方就是， 他設定的大多數規則的目的他並非要證實本身是正確的， 而是詳細的列出各規則的優缺點(pros and cons)，讓你更深入的理解這些規則。著名的統計學家 George E.P. Box 在數據建模中說過一句廣爲流傳的話： All models are wrong, but some are useful. 套用這句話到 code review 的 coding style 中， 我想說 All coding rules are wrong, but some are useful。固然 Google style 好的一個額外的緣由是 Google 提供了 cpplint 這個工具自動檢查你的 code 中是否有違反規範的地方， 大部分的低級錯誤都會被檢查出來。

3. 推廣 CodeReview

咱們小組剛開始把 Google 的 Rietveld 在內部搭建起來並在部門內推廣使用之後， 發現了一些問題， 主要包括：

1. issue 提交以後如何經過 email 發送到公司的 outlook 賬號
2. Rietveld 提供的提交 issue 的工具 upload.py 對中文的支持很差
3. 底層用的文本文件作存儲， 訪問量大的時候，提交 issue 多的時候， 速度不行

前面兩個問題charlieyan 和我對 Rietveld 的 python 源代碼作了一些修改， 很快解決了。 第3個問題咱們一直解決得很差。 不過當時來講支撐咱們幾個小組的開發仍是 OK的。 因此 yiwang 開始繼續推進 Rietveld 在 R2 的使用。 到了 2011 年的時候這套工具已經在 R2 的多個部門被使用了。 下面一張圖是咱們曾經統計過的各個部門當時的使用狀況

2010 年咱們在推廣 codereview 的時候， yiwang 也聯繫了研發管理部的同事， 也就是 code.oa.com 的維護人員， 當時 code.oa.com 並無嚴格意義的 codereview 功能。 它提供一個代碼 checkin 以後進行 codereview 的功能， 因此也沒有發送 codereview 的 upload.py 腳本；這個功能天然是有用的，可是過後的 review 每每有缺陷， 很難替代事前的 review。事實上更重要的問題是， 公司內部當時並無造成 codereview 的文化。 yiwang 和研發管理部的人交流的時候強烈的推薦了 Rietveld 系統， 不久以後code.oa.com 也仿照 Rietveld 開發了正式的 codereview 功能，而用於提交issue 的 tcr.py 也正是基於Rietveld 的 upload.py 作的修改。

而在搜搜內部，情況則有些不一樣。 Google 來的朱會燦帶領的搜索雲平臺(搜索基礎架構部）對 codereview 文化建設作了強力的推進， 基礎架構部門的幾個牛人開始使用這套系統以後， wujie, phong 他們接管了這套 CodeReview 系統， 當時上面提到的第三個問題當時已經成爲了一個嚴重的問題， wujie， phong 開始改進 Rietveld 的性能問題， 把底層的文本文件存儲替換爲 mysql 數據庫， 使用 apache 服務器替換了 Rietvled 基於 Django 的 HttpServer, 因此Rietveld 第三個和性能相關的問題也完全被解決了。 後續 huanyu 申請了 codereview.soso.oa.com 這個域名專門提供 codereview 服務， 因而直到搜搜和搜狗合併， 搜搜內部的好幾個部門的兄弟們一直基於 Rietveld 系統作 codereview。

4. C++ Readability

搜索廣告部門的情境廣告中心應該是我經歷的 code review 文化創建得最完全完善的地方。 當時 yiwang 是這個中心的總監，我和 huan 各負責一個小組， 咱們中心要負責開發一套 AFC(ads for content) 的情境廣告產品， 和如今廣點通有不少類似之處。 huan, yiwang, 包括咱們的 GM paulyan 都是 Google 的工程師， 而 AFC 這個產品幾乎是從零開始開發， 因此很天然的， 許多開發流程都是山寨的 Google 的， 雖然 AFC 這個產品最終因爲各類緣由並無在公司內部成功， 可是許多參與開發的工程師對於咱們當時創建起來的開發流程應該是印象深入的。 固然 codereview 是其中很重要的一個環節。 Huan 借鑑 Google 的CodeReview 流程， 在中心推進了一個 C++ readability 制度的創建。 一個語言的 readability 表示工程師對這個語言和相應的編碼規範的熟悉， 這個 C++ readability 最初只授予有限幾個高職級的工程師。 每個 codereview issue 至少須要獲得一位具備 C++ readabiltiy 的工程師的承認， 這個 issue 纔可以提交到代碼庫中。

而每個沒有 C++ readability 的工程師要想得到這個 readability, 須要提交一個 codereview issue 作專門的申請， 這個issue 的標題必須包含 「Apply for C++ Readability」, issue 至少包含三個文件， xxx.cc, xxx.h, xxx_test.cc, 若是這個 issue 經過嚴格的 codereview 被經過了， 就能夠授予這個工程師 C++ readability 。 而一般這種申請的 review 過程你們都會特別的認真細緻。

5. 在廣點通 CodeReview

2013年10月， 搜索廣告平臺部併入廣點通成立了SNG 的效果廣告平臺部， 原來AFC 的系統因爲這個合併事實上基本廢棄， 只是在開發過程當中不少模塊組件被逐步遷移到了廣點通的系統上。 咱們剛進入廣點通的時候， 廣點通使用 code.oa.com 平臺作 codereview, 也創建了一些 codereview 的文化， 可是有不少地方是執行不到位的。 coding style 雖然也推崇 Google coding style, 可是執行上不嚴格， 因此整個代碼庫的代碼風格是很不統一的。 這也給咱們的 codereview 執行帶來很大的挑戰：

• 舊的代碼風格不統一，咱們如何統一代碼風格， 舊的代碼是否須要基於新的風格重寫？
• 整個產品都在高速的開發迭代中，老大對於產品功能的開發演進有明確的指望， 產品經理每天在開發人員後面催進度， 咱們是否要執行嚴格的 code review ？

廣點通最後仍是明確的規定 coding style 統一到 Google C++ coding style， 咱們團隊的 CodeReview 也總體轉戰 code.oa.com, 咱們本身再也不使用 Rietveld（不過這套系統並無被廢棄， wujie 2014年一月的時候把這套系統搭建起來服務於微信， 並申請了域名 cr.oa.com), code.oa.com 的 CodeReview 功能通過幾年的改進， 也確實變得很好用。 考慮到產品迭代的節奏， 咱們廣告質量中心在推動的過程是漸進的：

• 全部新提交的文件必須使用 google style
• 全部新修改的代碼必須使用 google style
• 在指定的時間範圍內， 重要模塊的 code 必須重構爲 google style

爲了讓 codereview 文化在廣點通能更加完全的創建起來、執行到位， 幾位總監和架構師也迅速推進 code.oa.com 中實現了 code owner 的機制， 每一個目錄下面明確設置 code owner, 每一個change issue 要想提交， 必須有 code owner 經過才能夠。 而剛開始執行的時候， 廣告質量中心爲了保證 code review 執行到位， code review 是很是集權的， 全部相關目錄的 owner 只寫上總監。因此質量中心發出的全部 code review issue 只有總監經過了以後才能夠提交， 在必定的時間內這確實影響開發效率， 不過對教育全部開發人員的 code review 意識是高效的。 在經歷了近三週的嚴格控制以後， 目錄的owner 才加上組長， 而後逐步放開到組內的一些骨幹成員。

對於咱們小組而言， 有一半是以前 AFC的開發人員， 經歷過嚴格的 codereview 訓練； 有一半是廣點通原來的開發人員，沒有經歷過嚴格的 codereview 訓練。 爲了教育你們的 codereview 意識，在小組的週會上我要反覆的強調 codereview 的重要性；同時我明確的指出每一個人的考評是和 codereview 的表現相關的。

對於廣告業務的工程輸出主要包括項目中的設計文檔， 代碼實現， 線上A/B test實驗和實驗結果跟進分析， 以及平常在小組內的分享(日常積極的郵件討論，wiki 建設， 組內的 techtalk 分享等)。 全部的一線工程師， 不管職級高低，最重要的工程輸出原則仍是 show me the code, 而 codereview 是最可以反應這個客觀輸出的。 在小組內部， 咱們儘可能讓每一個人的 codereview 參與情況都公開透明， 因此咱們要求

• 每一個 codereview issue 要明確發送到你的項目合做者， 項目合做者 review 以後才容許提交
• 每一個 issue 都必須全員轉發到小組內全部成員， 小組內的任何人只要願意，均可以去review 其餘人的代碼

因爲全部的 codereview issue 都會發送到小組內全部成員， 因此我是可以在 code.oa.com 上看到全部人的 code 產出狀況的。我經過給你們作 codereview 判斷小組每一個工程師的工程輸出， code.oa.com 上大體是能夠看到小組內每一個人參與 codereview 的程度的， 包括他發出的 issue, 他給組內成員作的 codereview comments 等等。 code.oa.com 上提供了一個功能， 能夠 dump 出全部人的 codereview issue, 因此考評的時候我會檢查每一個成員在半年以內的 codereview 輸出情況，檢查組內成員提交的 issue 的質量。

不過 code.oa.com 上查看每一個人的 codereview 參與情況的功能如今仍是太弱， 若是code.oa.com 能提供一個統計分析功能, 使得每一個人能夠看到本身的一些統計指標， 包括

• 我修改的文件總數，修改和提交的代碼總行數
• 我給合做者作了多少次 codereview, 提供了多少次 comments
• 合做者給我作了多少次 comments

若是小組的組長可以看到組內成員的這些統計指標，無疑對於考評每個小組成員的工程輸出會提供不少客觀公正的信息。

6. CodeReview 感悟

作了4年的 CodeReview, 基本上這個流程在咱們的團隊已經深刻骨髓，成爲了小組工程工做中的種行爲習慣。 若是有一天我離開了騰訊， 我相信我在騰訊所經歷的相對嚴格的 CodeReview 訓練會幫我作好下一個項目的工程質量管理。 爲什麼要作 CodeReview 網絡上有不少的論證， 以前和 yiwang 合做的時候， 團隊內總結的幾點以下(主要都是 yiwang 總結的):

• Code Review保證開發質量，整個過程有章可循，無需顧及「面子」，可以修正不少平時不注意或者明知故犯的小毛病
• Code Review高效溝通交流，不需電話和視頻會議，讓北京深圳兩地的同事們一塊兒協同工做
• Code Review幫助知識傳遞，團隊內細粒度的知識分享， 你們一塊兒逐行的點評代碼比用技術交流會以及KM投稿更細緻
• Code Review保證代碼的可持續使用，代碼風格不一致致使一個系統每次移交負責人都被重寫一次
• Code Review和敏捷開發相容，而不是相斥，敏捷不是片面追求發佈次數，而是保證質量的發佈
• CodeReview 是一種社交的途徑， 鼓勵小組成員之間多作技術交流
• CodeReview 對新人的成長極其有利。 不少新人第一次作 codereview 的時候感受都是慘不忍睹， 被拍暈了， 我見過的最慘的 issue 被要求修改了20屢次以後才被容許提交。然而這種方式對新人的成長很是高效，一個coding 能力不錯的工程師， 幾個 codereview issue 以後， 很快就能寫出合格的代碼。
• CodeReview 增長團隊內代碼的可維護性, 同一份代碼至少會有兩我的熟悉——代碼的做者和審閱者。

如何建設一個團隊的 code review 文化， 跟隨 Google 的腳步， 也有一些具體的意見：

• 須要培養合格的reviewer。 創建各類編程語言的readability認證機制，從一組「種子」reviewer開始，讓更多同事得到認證成爲合格的reviewers
• 須要細化和公開各類語言的code style。 評註中能夠給出建議的出處的URL，使得保持代碼風格和可維護性落到實處
• 須要創建Code Lab機制。 幫助工程師們入門開發流程和規範
• 須要創建change approval機制。 鼓勵組內更多工程師改進代碼（敏捷），可是修改在提交前除了必須經過review，還須要相關責任人的approval；在加速開發滾動的同時，保證權責分明
• 引導輕量 review。咱們都經歷過有些工程師發一個 change issue 的時候， 幾十個文件一併發出來， 一個 issue 多是累積一週的修改， reviewer 看到這種 issue 幾乎都是崩潰的，不可能保證 review 質量。 因此好的 issue 應該是輕量的， 大的 issue 應該拆分爲小的 issue 發送。下圖是 Cisco 公司的codereview 報告中摘出的， 若是一個 issue 的修改代碼行數超過 400， 基本上 reviewer 是不可能認真 review 幫你找出 bug 的 。

• 明確issue 可否提交是工程師本身的責任。開發人員有時候抱怨別人 review 得慢， 拖延時間。 咱們小組內部一般明確指出：一個 issue 可否提交是開發人員本身的責任。 開發人員本身要積極推進合做人員幫忙 review issue， 推進 issue 被經過並及時提交。

7. 結語

學術界的全部高質量論文都是 peer review 以後纔有可能發表的。 寫過 paper 的碩士博士都有一個體會， 接到的各類 review 意見有時候令你很是的痛苦， 然而不管是你拒絕了 reviewer 的意見，仍是接受意見修改了文章， 你都會經歷一個認真思考的過程去提高你文章的質量。 高質量的論文是科研人員嚴格 review 出來的， 我相信高質量的 code 也能夠經過工程師的 review 產生。 把CodeReview 打形成團隊的習慣，而咱們逐漸會發現這種習慣所釋放出來的正能量。 但願有一天, 咱們也能作到：

At Tencent, no code, for any product, for any project, gets checked in until it gets a positive review。