1. 為什么要為一個看起來誰都懂的事寫套方案先講一件讓我徹底改變對代碼審查看法的舊事。三年前我們上線一個訂單狀態(tài)同步服務(wù)代碼評審記錄里干干凈凈兩位 reviewer 都點了 approve。結(jié)果上線第四天線上出現(xiàn)了一整批訂單違約金計算錯誤凌晨兩點被監(jiān)控電話叫醒。我打開那條已經(jīng)合入的提交問題一目了然一個 for 循環(huán)在集合為空時直接返回了默認值 0而業(yè)務(wù)上這時應(yīng)該拋錯并觸發(fā)告警。這條提交通過了所有人工審查因為所有人都默認給別人看的代碼應(yīng)該沒問題而那個提交恰好看起來好像沒問題。排查到凌晨五點多的時候我跟搭檔說了一句至今記憶猶新的話我們不是沒有代碼審查我們是有的審查比沒有更危險——它給了所有人一個已經(jīng)確認過安全的錯覺。后來我很認真地研究了一圈業(yè)界的東西也參考了 Google 那篇經(jīng)典工程實踐文檔里關(guān)于 code review 的章節(jié)再結(jié)合自己在多個團隊推行的經(jīng)驗把整套做法沉淀成了一個叫open-code-review的開源方案。它不是一個插件也不是一個 SaaS 工具而是一整套關(guān)于怎么讓代碼審查在一個團隊里真正生效的開放方法論配套了分支保護配置、Pull Request 模板、審查清單、評審人指派機制和指標采集腳本。不管你是三五人的小團隊還是幾十上百人的技術(shù)組織這套方案里的東西基本都能直接抄走用。這篇文章我會把核心內(nèi)容攤開來講包括我踩過的坑和從數(shù)據(jù)里看到的現(xiàn)象希望能幫你的團隊把代碼審查從走形式變成真防線。2. 審查流程為什么常常淪為橡皮圖章以及怎么從入口處堵住2.1 橡皮圖章審查的心理機制很多團隊不是不重視 code review而是審查的質(zhì)量長期處于假性有效狀態(tài)。approve 按鈕按下去的成本太低了低到 review 一個 PR 的時間可能比刷一條短視頻還短。但真正的問題不是成本低而是認知上的錯覺審查者默認作者已經(jīng)自測過了作者默認審查者會幫我把關(guān)。兩個默認一疊加所有提交都變成了一種無人真正負責的流程體操。Google 的工程實踐文檔里提到過一個很重要的觀點代碼審查的主要目的不是找 bug而是保證代碼和整個代碼庫處于一致、可持續(xù)維護的狀態(tài)。這個定位差別非常大。如果你把審查的目標只定義成找 bug那審查者確實容易懈怠因為絕大多數(shù)改動根本沒有明顯 bug 可找。但如果你把審查目標定義成讓每個進入主干的分支都對得起后續(xù)維護它的人那審查的內(nèi)容和標準就完全不一樣了。open-code-review的第一步就是在流程入口處逼著所有人改變這個心理預期。2.2 入口收緊分支保護與提交粒度控制一個代碼庫如果要長期維護主分支應(yīng)當是受保護的這個沒什么爭議。但保護到什么程度不同團隊的差異就大了。我們當時在主分支上強制了四種必過檢查最新代碼拉取、自動化測試全綠、靜態(tài)檢查零錯誤、至少兩個符合條件的 reviewer 顯式 approve。前三條靠 CI 就能做到制度化最難的是最后一條。配置分支保護本身不難我這里直接給一份在主流 Git 托管平臺上通用的規(guī)則模板禁止直接向主分支推送代碼所有變更必須通過 Pull Request 合入要求 PR 內(nèi)所有對話comments必須 resolve 才能合入要求至少 2 個 approve且批準者不能是作者本人要求 CI 里的構(gòu)建、測試、靜態(tài)檢查全部通過要求分支與主分支保持同步防止合并時引入舊代碼覆蓋新邏輯這些規(guī)則看上去平平無奇但它們組合起來的力量在于它把碰運氣型審查變成了過五關(guān)型審查。即使某個環(huán)節(jié)真的是敷衍的至少還有別的環(huán)節(jié)在做事實兜底。隨后是 PR 的粒度控制。這是我覺得比分支保護更影響審查質(zhì)量的一件事。一個超過一千行改動的 PRreviewer 看完前三百行就已經(jīng)開始走神了剩下的部分基本只會看個大概。這不怪任何人人類的工作記憶本來就沒法長時間維持對陌生代碼的高強度注意力。所以我們在open-code-review里定了一條硬性參考標準一個 PR 的凈改動盡量控制在 200 到 400 行之間。超過這個范圍建議拆分成多個有依賴順序的 PR 提交。這條規(guī)矩在推行初期阻力不小很多人都覺得拆分 PR 好麻煩一次改完多痛快。但從實際效果看一旦 PR 變小reviewer 的評論質(zhì)量肉眼可見地上升了不再只是LGTM或有一個小問題請改一下而是能明確說出你在某個邊界條件下漏掉了錯誤處理這種具體結(jié)論。2.3 審查人指派輪值制與專家制的平衡誰來做 reviewer是一個經(jīng)常被忽略但直接影響審查質(zhì)量的設(shè)計。很多團隊的默認做法是誰創(chuàng)建的 PR 就隨機或者就近拉一兩個人來看。隨機帶來的后果是后端的人給你審前端代碼資深的人給新人兜底但自己真正擅長的領(lǐng)域沒人管。我們最后采用的是混合制指派方式適用場景優(yōu)點缺點模塊負責人強制核心模塊、基礎(chǔ)設(shè)施變更領(lǐng)域?qū)＜野殃P(guān)質(zhì)量可靠模塊負責人可能成為瓶頸輪值審查日常業(yè)務(wù)迭代、低風險改動人人參與知識面擴散輪值者可能不熟悉模塊上下文興趣認領(lǐng)團隊周知、公告類 PR參與感強無法保證有人認領(lǐng)每個 PR 至少要有兩個 reviewer通常一個是熟悉這塊業(yè)務(wù)的領(lǐng)域人負責邏輯正確性和邊界條件另一個是站在全局視角的架構(gòu)人負責可維護性、命名、抽象層次和是否引入了重復代碼。雙人組合的初衷很樸素一個人看樹一個人看林總有一個會發(fā)現(xiàn)問題。但要注意這必須是顯式指派而不是大家有空就來看看。這套機制剛落地的時候大家有點不適應(yīng)覺得繁瑣。但堅持跑了兩三個迭代之后一個直接可感知的變化是合入主干后需要緊急修復的問題明顯變少了。代碼審查從來不是單點技巧的問題它是一個系統(tǒng)工程入口處的設(shè)計決定了后面所有環(huán)節(jié)的有效性上限。3. 可執(zhí)行的審查次序從 diff 順序到先理解后判斷3.1 為什么 review 的順序比速度重要很多有經(jīng)驗的工程師在審代碼時習慣從第一個文件開始往下看。這個習慣在文件少的 PR 里沒問題但一旦 PR 動到了多個模塊線性閱讀就很容易陷入只見樹木不見森林的困境。open-code-review里給出的建議是按照先高后低的層次來讀 diff先看 PR 描述和關(guān)聯(lián)的 issue弄清楚這個改動想解決什么問題再看測試理解作者期望的輸入輸出行為是什么然后看接口或函數(shù)簽名層面的變化判斷抽象邊界是否合理最后才深入具體實現(xiàn)檢查邏輯細節(jié)這套順序的核心邏輯是你首先要理解這個改動為什么存在然后才能判斷它做得對不對。如果一上來就鉆進實現(xiàn)細節(jié)很容易被局部技巧吸引注意力忽略掉整體設(shè)計上的問題。有一個真實案例很說明問題。一位同事的 PR 改造了內(nèi)部的消息隊列消費邏輯把原先同步確認改成異步確認目的是提高吞吐。如果按文件順序往下看會先看到消息處理函數(shù)里的重試代碼寫得確實漂亮。但當時一個 reviewer 先看了測試發(fā)現(xiàn)測試里的異步場景根本沒有覆蓋進程在確認之前崩潰的情況于是他回頭去查實現(xiàn)很快就確認了這是語義級別的漏洞推進了修復。如果沿著文件從頭讀到尾這個問題大概率會被淹沒在代碼很精致的印象里。3.2 審查請求里強制要有的三類信息open-code-review的 PR 模板里明確要求每個 PR 在描述區(qū)帶上三個部分改動動機、影響面、測試方法。這三類信息缺失的 PR 直接打回不進入 review 階段。改動動機不是把 issue 標題復制一遍而是要說明為什么必須這樣改。這個要求會讓作者在做變更之前先想清楚。影響面則要求作者標記出本次改動涉及哪些模塊、是否有數(shù)據(jù)遷移、是否有配置變更、是否需要回滾預案。測試方法要求寫明人話層面的驗證比如本地起了一個三個節(jié)點的集群驗證了消息不丟失。這三類信息的強制化有兩個作用。第一它逼著作者在點擊創(chuàng)建 PR 之前先自我檢查一遍很多低級問題在這一步就被攔截了。第二它大幅降低了 reviewer 的閱讀理解成本reviewer 不需要在代碼里猜作者的意圖腦子里省下的認知資源全部可以用在真正的質(zhì)量判斷上。我見過不少團隊在推行這項模板時擔心增加作者的負擔但從實測看認真填這三段的 PR整體返工次數(shù)反而更少因為作者在填模板時暴露出來的思路偏差往往比 reviewer 看完代碼提的兩三條評論更早地糾正了方向。3.3 自查階段作者提交前先把 diff 讀一遍有一件事幾乎不需要任何成本卻經(jīng)常被忽略那就是作者在提交 PR 之前先以 reviewer 的心態(tài)把自己生成的 diff 從頭到尾讀一遍。這個習慣第一次是我在一個開源項目貢獻代碼時被維護者要求的。當時我在提交前讀了一遍自己的 diff至少發(fā)現(xiàn)三處問題一處是忘記刪掉的調(diào)試日志一處是復制粘貼出來的重復代碼塊還有一處是命名完全語義不符的臨時變量。這些東西如果直接發(fā)出去reviewer 需要在評論里逐一提浪費雙方時間。open-code-review在這方面給出的要求很具體作者提交 PR 前自己對變更做一次冷讀像第一次看陌生代碼一樣至少檢查注釋是否有誤導、調(diào)試代碼是否殘留、是否有比當前實現(xiàn)更簡單的寫法三個點。這十幾分鐘的自查投入換來的是 PR 被 reviewer 秒批的概率大幅度提升。4. 審查清單把經(jīng)驗變成可驗證的條目4.1 為什么清單比天賦可靠很多優(yōu)秀工程師的審查能力來自常年積累的經(jīng)驗但經(jīng)驗的問題是它不可被復制也不容易被量化。一個團隊里如果只有一個具備毒辣眼光的資深大佬那大佬請假的時候?qū)彶橘|(zhì)量就會顯著下滑。open-code-review的做法是把經(jīng)驗拆解成清單讓普通工程師也能按圖索驥地完成高質(zhì)量的審查。這個思路其實是從航空業(yè)借鑒來的飛行員的 checklist 不是一個智商測試而是確保在高認知負荷下仍然不遺漏關(guān)鍵項的行為約束。我們沉淀的審查清單總共分五個維度每個維度下有幾條關(guān)鍵的驗證行為。在open-code-review倉庫里這份清單被維護成 Markdown 文件且允許團隊按自己的項目類型增刪條目。4.2 五個核心維度互相之間的關(guān)系程序正確性是最基礎(chǔ)的維度但它的覆蓋面非常廣遠不止邏輯對不對。實際審查時至少要覆蓋數(shù)據(jù)為空、集合為空、字符串為空時代碼是否做了符合業(yè)務(wù)語義的處理并發(fā)場景下是否有競態(tài)條件或死鎖隱患重復調(diào)用是否具備冪等性超時和錯誤路徑發(fā)生時是否有顯式的失敗信號而不是靜默吞掉是否有防御性代碼存在但邏輯錯誤比如if (a b)寫成if (a || b)代碼可維護性審查關(guān)注的是抽象層次和依賴方向。一個長期可維護的代碼庫應(yīng)該是高層策略依賴底層接口而不是反過來。審查者要問的是這個改動是否讓模塊之間的依賴關(guān)系變得更亂是否出現(xiàn)了為了省事而直接在業(yè)務(wù)層調(diào)用底層存儲類的情況是否引入了一個看起來能解決當前問題但會限制未來擴展的設(shè)計可測試性也是一個獨立維度。代碼在提交前是否配套了該有的單元測試測試是驗證了真實行為還是只是讓覆蓋率數(shù)字好看很多項目的測試越寫越表演化只覆蓋主流程、不覆蓋異常分支本質(zhì)上是因為作者覺得有測試這個動作比較體面而不是真的想用測試守住工程質(zhì)量。對這個現(xiàn)象我們會在清單里明確要求 reviewer 檢查測試斷言是否有效比如一個測試是否真的會失敗——如果一個測試去掉斷言仍然能通過那它就不該存在。性能與安全維度容易被非專業(yè)領(lǐng)域的人忽略比如N1查詢問題、循環(huán)內(nèi)的耗時操作、拼接 SQL 的入口是否經(jīng)過參數(shù)化、敏感信息有沒有被打印到日志里。這一類問題單靠 reviewer 的領(lǐng)域經(jīng)驗很難全覆蓋所以我們在清單里以高頻問題集的形式沉淀了常見的性能和安全反模式。4.3 一份可以直接抄走的清單模板下面這份清單是從open-code-review里摘出來的精簡版5 個維度每維度 6 到 8 項適合大多數(shù)業(yè)務(wù)后端項目維度關(guān)鍵檢查項正確性邊界條件處理、異常路徑、并發(fā)安全、冪等性、日志是否輸出有效上下文可維護性函數(shù)是否有單一職責、命名是否表意、是否存在復制粘貼的重復邏輯、依賴方向是否合理可測試性是否有對應(yīng)的測試、測試是否覆蓋關(guān)鍵分支、斷言是否有效、測試命名是否描述了期望行為性能與安全是否出現(xiàn)循環(huán)內(nèi) IO、是否存在 N1 查詢、敏感信息是否被記錄、外部輸入是否有校驗兼容性與遷移數(shù)據(jù)庫遷移是否可回滾、配置變更是否向后兼容、有無破壞 API 契約、有無灰度開關(guān)這份清單不需要每次審查都全部硬過一遍。對于超低風險改動比如改一個文案、加一個前端字段reviewer 可以只跑其中兩三個維度。但清單的存在價值在于當一個改動碰觸了敏感模塊時reviewer 可以根據(jù)清單逐項核驗而不是憑感覺給看起來差不多可以的結(jié)論。我個人的經(jīng)驗是把這份清單打印出來貼在顯示器旁邊連續(xù)堅持兩個月審查時的腦補環(huán)節(jié)會大幅減少評論質(zhì)量會明顯提升因為人一旦知道自己要驗證什么就不會只用感覺下判斷。5. 分歧處理與審查數(shù)據(jù)讓流程活下來的關(guān)鍵機制5.1 技術(shù)分歧的本質(zhì)和收斂方式代碼審查永遠繞不開人而人一多分歧就不可避免。很多團隊最后的式微不是死于沒有流程而是死于分歧處理不當要么權(quán)威壓制導致新人不敢說話要么為了和氣什么都放行。open-code-review里收斂分歧的原則只有三條正確性問題用事實說話風格偏好引用團隊規(guī)范雙方爭執(zhí)超過 15 分鐘拉第三個人或升級討論先解釋第一條。如果一個 reviewer 說這里會拋異常而作者說不會拋異常那這個問題根本不是辯論出來的是用代碼事實來驗證的。可以要求作者補一個測試來證明當前行為或者 reviewer 直接跑一下復現(xiàn)場景。把分歧落到可驗證的實驗上是效率最高的解決方式。第二條針對的是那些沒有對錯之分的風格問題。代碼里具體是函數(shù)式寫法還是命令式寫法新的對象是配置注入還是直接 new本質(zhì)上沒有絕對正確答案。這時唯一的依據(jù)是團隊已經(jīng)約定的規(guī)范文檔。沒有規(guī)范怎么辦那就把分歧作為一條新規(guī)范提到團隊例會討論而不是當當場攻訐。第三條是最容易被忽視的。兩個資深工程師對同一個抽象方案各有堅持誰都覺得自己才是對的這種爭執(zhí)持續(xù)半小時以上時繼續(xù)爭下去只會消耗雙方精力。此時應(yīng)該由初始審查人引入一個項目組外但對架構(gòu)有判斷力的人或者把兩種方案分別做成小原型用代碼量、后續(xù)擴展成本等硬指標來定結(jié)論?；仡櫸疫@邊多年碰到的歷史分歧幾乎每一個傷感情的案例都是緣于把風格偏好問題無限上升為正確性問題或者把正確性問題無限擱置為風格問題兩個方向都不健康。5.2 采集什么指標以及怎么避開 KPI 陷阱把審查跑起來之后下一步會自然產(chǎn)生的需求是如何衡量這套機制到底有沒有用指標一定要采集但要小心指標一旦變成 KPI就會立刻失真。我們實際記錄并且長期跟蹤的指標有四個審查耗時一個 PR 從創(chuàng)建到合入經(jīng)過多長時間評論密度每 100 行有效評論數(shù)量去除LGTM、空行這類無內(nèi)容評論平均修改輪次一個 PR 從提交到合入之間作者收到幾輪修改意見缺陷逃逸率合入后線上發(fā)現(xiàn)的問題回溯時有多少是曾經(jīng)被 review 過的提交引入的這四個指標里前三個用來衡量過程的健康度最后一個用來衡量結(jié)果的收口程度。它們之間的聯(lián)系值得反復觀察如果評論密度高但修改輪次也高說明過程中抓出不少問題這是正常的如果評論密度低但缺陷逃逸率高說明 reviewers 大概率在走過場需要警惕。我特別建議不要做的一件事是把評論數(shù)量直接掛鉤績效。很早我試過一次把每個人當月的 comment 數(shù)作為積極性的參考指標結(jié)果不出一個月就開始有人在不該發(fā)言的場合也強行發(fā)言為了評論而評論評論區(qū)被垃圾信息淹沒。復盤之后我們立刻取消了這一指標只把它作為一種團隊內(nèi)部匿名可見的參考數(shù)據(jù)效果反而回到正常。5.3 用線性總結(jié)讓 flow 持續(xù)迭代open-code-review最后收口在這個機制上每季度對 review 數(shù)據(jù)進行一次小的復盤把線上缺陷樣本回填到審查清單里作為下季度的新增檢查項。比如上面提到的線上問題如果發(fā)生在你們團隊季度復盤時就會在正確性清單里新增一條空集合或空結(jié)果集時要有顯式的錯誤信號而非默認值。下一個季度的 review所有人都會被這條新規(guī)則約束住。如此反復迭代下去清單會越來越貼近這個團隊真實踩過的坑。有一個隱含的好處是新人對這套流程的適應(yīng)成本很低。他不需要在入職前就擁有很多年經(jīng)驗只要老老實實按照清單過完就能做到大部分人做不到的仔細程度。審查經(jīng)驗從此從個人天賦變成了組織能力技術(shù)最強的人審得最準不再是團隊的脆弱依賴點。用一句話總結(jié)我這幾年圍繞 open-code-review 最大的收獲代碼審查要解決的核心問題不是找 bug而是讓團隊對代碼質(zhì)量產(chǎn)生統(tǒng)一的、可持續(xù)的認知。流程、清單、指標、分歧處理機制全部服務(wù)于這個認知的建立。每個人看代碼的視角天然不同但有了這套框架所有視角都在朝同一個方向用力這是它和我之前見過的所有高復雜度審查工具在本質(zhì)上的區(qū)別。