提到“Uber uReview代碼審查”這個項目名很多人的第一反應(yīng)是Uber不是做打車的嗎怎么跑去搞代碼審查工具了其實Uber內(nèi)部的技術(shù)氛圍一直很濃很多工程實踐和工具鏈都相當(dāng)扎實uReview就是他們開源出來的一套偏工程化的代碼審查輔助方案。代碼審查這件事幾乎所有團隊都在做但真正做得好的沒幾個uReview的核心思路是把“審查規(guī)則”變成倉庫里可維護的配置讓每一次Pull Request的審查都走同一條標準流程減少reviewer憑感覺發(fā)揮的空間。如果你帶過團隊、做過技術(shù)負責(zé)人或者只是單純想把自己倉庫的審查流程規(guī)范化這個項目值得研究一下。這篇就從一個實際使用者的角度聊聊uReview的設(shè)計思路、部署方式以及我踩過的坑。1. 為什么代碼審查這么難做好1.1 代碼審查的真正難點代碼審查這套東西,說簡單也簡單,說難是真難。簡單的地方在于,看一個人的PR然后給反饋,誰都會;難的地方在于,不同的評審人看同一個PR,給出的意見可能天差地別。有的reviewer死磕命名,一個變量名能來回討論好幾輪;有的reviewer只關(guān)心主流程有沒有bug,其他一概不管;還有的reviewer干脆只回一個LGTM,連代碼都沒細看。這種差異帶來的直接后果就是:審查質(zhì)量完全取決于這次PR碰巧分配到誰手里。同一個改動,遇到嚴格的reviewer可能被打回重寫,遇到寬松的reviewer可能一次就過了。團隊里一旦出現(xiàn)這種情況,開發(fā)體驗會很差,而且很容易引發(fā)矛盾——你為什么這么針對我我上次這么寫不是也過了嗎。我自己待過幾個團隊,這個問題幾乎無解。定了審查規(guī)范也沒用,規(guī)范是文檔,文檔是沒人看的。真正能讓規(guī)范落地的,要么靠人肉提醒,要么靠工具強制執(zhí)行。uReview走的就是第二條路。1.2 Uber碰到的實際問題Uber在2017年前后重點建設(shè)支付業(yè)務(wù)的時候,是出了幾次線上事故的。事后復(fù)盤的時候發(fā)現(xiàn),很多問題根子上都出在代碼審查這個環(huán)節(jié)——不是代碼本身多難寫,而是提交上來的代碼經(jīng)常帶著明顯的基礎(chǔ)性缺陷,比如:改了數(shù)據(jù)庫遷移腳本,但是忘了同步更新回滾邏輯新增了依賴庫,但是沒有檢查license是否合規(guī)改了API接口,卻沒有更新對應(yīng)的文檔或者客戶端SDK影響線上交易核心鏈路的改動,根本沒有評估過灰度方案這些問題單看技術(shù)含量都不高,但架不住多。每個PR都靠reviewer人肉去查,漏掉一個就是一顆雷。Uber的做法是把這些檢查點固化成清單,每個PR都必須逐項確認,確認不了就不許合入。這個思路一開始是在支付團隊內(nèi)部用文檔人工勾選的方式試的,后來覺得太麻煩,就順手寫了個工具把它自動化了,這就是uReview的雛形。1.3 uReview到底解決了什么uReview解決的問題可以概括成一句話:把審查流程中“非主觀判斷”的部分用機制固化下來。代碼風(fēng)格好不好、架構(gòu)設(shè)計合不合理,這類東西uReview管不了,那是reviewer的經(jīng)驗和水平問題。但“是否包含測試”“是否有數(shù)據(jù)庫遷移”“是否修改了依賴”“是否需要更新文檔”這類問題,是可以通過規(guī)則強制檢查的。工具的做法很簡單:你在倉庫里放一個規(guī)則配置文件,uReview讀取這個文件,把里面的每一條規(guī)則對應(yīng)到GitHub PR評論里的一個checkbox。reviewer打開PR,評論區(qū)頂部就是一份自動生成的審查清單,每條規(guī)則后面都有一個可以勾選的框。規(guī)則是寫進倉庫代碼里的,和代碼一起評審、一起變更,所有人都能看到,而不是躺在某個角落里吃灰的wiki文檔。2. uReview的核心設(shè)計:規(guī)則即代碼2.1 rules.yaml的結(jié)構(gòu)拆解uReview的使用核心是一個YAML配置文件,官方示例里一般叫rules.yaml。整個文件的結(jié)構(gòu)非常直觀,頂層是倉庫級別的配置,然后往下就是一條一條的規(guī)則。每條規(guī)則由兩部分組成:type和items。type定義了這條規(guī)則的類別,目前支持的主要有required、support、informational。required是強制項,審查人必須確認沒有問題才能勾選;support是建議項,允許審查人跳過,但至少要過目一眼;informational就是純提示,不需要審查人做任何操作,只是把信息帶到PR評論里。items是規(guī)則的具體清單項,每一項都是一個字符串描述,比如“確保所有異常都記錄了日志”。當(dāng)uReview跑起來的時候,它會把這個結(jié)構(gòu)轉(zhuǎn)換成PR評論里的檢查列表,每條item對應(yīng)一個checkbox。repo: my-project rules: - type: required items: - Change includes tests - Change includes migration or rollback plan - type: support items: - Update README if user-facing behavior changed上面這個例子就定義了一條強制規(guī)則(必須包含測試、必須包含遷移或回滾方案)和一條建議規(guī)則(如果影響了用戶可見行為,需要更新README)。實際用下來,rules.yaml的寫法沒有任何黑魔法,就是一個普通的結(jié)構(gòu)化配置,團隊成員一看就懂。2.2 uReview檢查哪些內(nèi)容uReview本身不能被理解成一個靜態(tài)代碼分析工具。它不做lint、不跑測試、不掃漏洞,它做的事情可以理解為“上下文感知的審查助手”——根據(jù)PR涉及的文件和改動范圍,決定哪些規(guī)則需要被激活,然后生成一份定制化的檢查清單。具體來說,uReview在運行時主要做這幾件事:獲取當(dāng)前PR的元信息,包括標題、描述、變更文件列表、當(dāng)前狀態(tài)讀取倉庫根目錄下的rules.yaml如果在PR描述里發(fā)現(xiàn)了特殊指令,比如跳過某些規(guī)則的標注,會識別并記錄在PR上更新一條評論,把規(guī)則轉(zhuǎn)成checklist展示出來更貼心的一點是,uReview支持在規(guī)則描述里寫一些模板變量,可以根據(jù)PR的實際信息動態(tài)生成描述。比如一條規(guī)則可以寫成:“本次變更涉及{changed_files}個文件,請確認數(shù)據(jù)庫遷移腳本已同步更新”。這樣生成的checklist就帶上了上下文信息,reviewer不用再去手動數(shù)文件,看起來更直觀。2.3 為什么用GitHub PR評論而不是獨立網(wǎng)站這里其實是一個很關(guān)鍵的產(chǎn)品決策。uReview完全可以做成一個獨立的web服務(wù),給每個PR生成一個審查鏈接,reviewer點進去勾選。但Uber選擇了直接復(fù)用GitHub的PR評論作為交互載體。原因很簡單:降低切換成本。工程師的日常工作流里,Review PR的活動本來就發(fā)生在GitHub上,如果還要額外去另一個網(wǎng)站打開一個審查頁面,這中間就多了一次上下文切換。而把checklist直接放在PR評論區(qū),reviewer打開PR的同時就能看到全部規(guī)則,勾選也直接通過GitHub的評論操作完成,不離開當(dāng)前頁面,幾乎沒有適應(yīng)成本。這個設(shè)計思路其實很值得借鑒。很多團隊做工具的時候喜歡造一個新入口,要用戶打卡、登錄、進入專屬工作臺,聽起來很酷,但用戶根本不會用。uReview把工具“放進”用戶已經(jīng)在使用的環(huán)境里,這種嵌入式的體驗反而更容易被接受。3. 從頭到尾接入uReview的完整過程3.1 前置條件說明如果你想在自己的項目里用uReview,需要準備幾樣?xùn)|西:一個GitHub倉庫、一個能跑GitHub Actions的CI環(huán)境,以及一個具有PR讀寫權(quán)限的GitHub訪問令牌。如果你們用的是GitHub免費版,只要倉庫是公開的,Actions的免費時長足夠跑uReview這種輕量級任務(wù);如果是私有倉庫,就需要確認一下套餐里包含的Actions分鐘數(shù)。除了GitHub之外,uReview官方實現(xiàn)里也有對GitLab等平臺的支持計劃,但實際成熟路徑還是GitHub為主。所以如果你公司的代碼托管在GitLab上,接入成本會略高一些。不過據(jù)我了解,目前網(wǎng)上也有一些基于uReview思路的自制適配方案,把同款checklist邏輯移植到GitLab MR上,這種屬于二次開發(fā)了,不在原版能力范圍內(nèi)。3.2 在倉庫里添加rules.yaml接入的第一步是在項目根目錄創(chuàng)建rules.yaml文件。這里我給一個更完整的實際案例,以一個小型后端服務(wù)為例:repo: payment-service rules: - type: required items: - Change includes unit tests for new logic - All error cases are handled or explicitly documented - Database migration has a rollback plan - No new dependency without license verification - type: support items: - Update API documentation for endpoint changes - Add metric or logging for new critical paths - type: informational items: - This PR includes changes to the payment module, please double-check寫這個文件的時候有幾個點需要留意。第一,規(guī)則不要寫太多,每個type下3到5條就差不多了。見過有團隊把規(guī)則堆到20多條,結(jié)果reviewer根本沒耐心逐條看,最后全變成走過場,還不如不搞。第二,規(guī)則描述要寫清楚,一句話說清楚“要做什么”和“為什么”,不要出現(xiàn)類似“請檢查代碼質(zhì)量”這種空洞的表述。第三,informational類型默認不需要勾選,但描述里最好帶上具體的預(yù)警點,比如哪些模塊被改動過、有什么風(fēng)險,這樣才有信息價值。3.3 通過GitHub Actions觸發(fā)uReview配置文件準備好之后,就需要讓uReview在每次PR打開或者更新時自動運行。這里用的是GitHub Actions,workflow文件大致長這樣:name: uReview on: pull_request: types: [opened, synchronize, reopened] jobs: ureview: runs-on: ubuntu-latest steps: - name: Checkout code uses: actions/checkoutv4 - name: Run uReview uses: uber-common/uReviewv1 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}這個workflow的trigger事件是pull_request,并且在PR新開、代碼更新、重新打開三種情況下都會執(zhí)行。GITHUB_TOKEN用的是GitHub Actions內(nèi)置的自動令牌,不需要額外配置secrets,只要倉庫的工作流權(quán)限允許寫PR評論就行。如果權(quán)限設(shè)置有問題,可以在倉庫Settings - Actions - General - Workflow permissions里勾選“Read and write permissions”。跑完一次之后,你打開對應(yīng)的PR,評論區(qū)底部就會出現(xiàn)一份uReview生成的checklist。后面的reviewer流程就變成這樣:打開PR,看代碼,對照checklist逐項確認,有問題的直接在評論里指出,沒問題就把對應(yīng)的checkbox勾掉。等所有規(guī)則都被確認之后,PR才允許合入。3.4 如何強制規(guī)則必須被確認防止reviewer漏掉檢查項的機制,uReview是通過狀態(tài)檢查(status check)來實現(xiàn)的。默認情況下,uReview規(guī)則在PR評論里展示后,reviewer是不是真的勾了,uReview本身不會攔截合入。如果你希望做到“不確認就不許合入”,需要結(jié)合GitHub的branch protection rules。你可以在倉庫的Settings - Branches里為main分支添加一個保護規(guī)則,把uReview對應(yīng)的status check設(shè)為required。這樣的話,只要uReview還沒有跑完,或者規(guī)則里的required類型沒有被全部勾選,PR就一直處于“checks未通過”的狀態(tài),合入按鈕是灰的。我團隊實際用下來,這種“強制性”特別重要。規(guī)則寫出來,如果只是發(fā)一條評論提示一下,那它跟普通評論沒有區(qū)別,reviewer該跳過還是跳過。只有把規(guī)則變成合入的前置條件,大家才會認真對待。這一點在整個uReview的設(shè)計哲學(xué)里占了很重的分量。4. 常見問題與排查技巧實錄4.1 規(guī)則沒有出現(xiàn)在PR評論里這是接入uReview時最常碰到的問題。workflow正常運行、Actions日志里也沒有報錯,但PR評論區(qū)就是看不到uReview生成的checklist。排查下來發(fā)現(xiàn),大部分情況跟actions/checkout的版本有關(guān)。如果checkout版本過舊,它默認只拉取當(dāng)前PR的合并后代碼,不會包含目標分支的最新狀態(tài)。uReview在判斷倉庫配置的時候,就有可能讀到舊版本的rules.yaml,或者讀不到文件,從而選擇靜默跳過。解決方法是把checkout升級到v4,并明確指定fetch深度:- name: Checkout code uses: actions/checkoutv4 with: fetch-depth: 0另外還有一種情況:rules.yaml文件名大小寫不對。Linux環(huán)境下的文件名是區(qū)分大小寫的,如果你創(chuàng)建的其實是Rules.yaml,uReview在查找rules.yaml的時候就找不到?？梢韵仍诒镜赜胠s確認一下文件名。4.2 評論更新了但之前的勾選狀態(tài)丟了uReview每次在PR更新時運行,都會刷新一次評論內(nèi)容。如果reviewer已經(jīng)在舊版評論里勾選了一些checkbox,新評論生成后,這些勾選狀態(tài)并不會自動保留。這在代碼持續(xù)迭代的PR里會造成一點點體驗上的摩擦——你剛勾完4項,對方push了一個小修改,uReview重新跑一遍,之前的勾都沒了。官方目前的策略就是直接覆蓋評論,不遷就舊的勾選狀態(tài)。我的建議是,如果PR改動比較大、需要多輪review,盡量讓開發(fā)者在分支上少做瑣碎的小修改,攢一批push一次;或者reviewer先把意見留在代碼行級評論里,最后確認的時候再統(tǒng)一走uReview的checklist。這樣勾選次數(shù)少一些,狀態(tài)丟失的影響也就有限。4.3 Status check狀態(tài)與rules不匹配接入branch protection之后,有些團隊會遇到一種奇怪現(xiàn)象:uReview已經(jīng)跑成功了,狀態(tài)檢查也顯示綠色通過,但PR合入按鈕還是被攔截。這種問題多數(shù)時候是因為保護規(guī)則里指定的check name和uReview實際上報的check name不一致。GitHub的status check匹配看的是名稱,你可以在uReview的日志里找到它上報的check name,比較常見的就是uReview。如果分支保護規(guī)則里填的是uReview / job-name,那就可能對不上。解決方法是在保護規(guī)則里把狀態(tài)檢查的名稱改成日志里顯示的那個,或者直接重新選擇列表里已有的check。4.4 規(guī)則老化后失去意義uReview用得越久,規(guī)則列表越容易出現(xiàn)“僵尸規(guī)則”。比如團隊已經(jīng)明確不兼容某個老框架了,但rules里還有一條“確認兼容舊框架”的規(guī)則;又比如某個模塊已經(jīng)被廢棄,但跟該模塊相關(guān)的提醒規(guī)則還掛在informational類型下,每天出現(xiàn)在所有PR評論里。時間一長,reviewer對規(guī)則會逐漸麻木,本來有意義的規(guī)則也會被當(dāng)成噪音。建議每隔一兩個季度做一次規(guī)則體檢,把rules.yaml里的每一條規(guī)則都過一遍,問一個問題:這條規(guī)則現(xiàn)在還能擋住真實問題嗎?擋不住的,刪。擋得住的,想想能不能用自動化方式替代,能替代的就減掉,讓reviewer把有限的注意力留給機器判斷不了的東西。問題類型癥狀解決方案配置文件缺失或名稱錯誤PR無評論,uReview日志無報錯確認rules.yaml文件名、位置是否正確checkout讀取到舊代碼評論內(nèi)容與最新配置不符升級checkout到v4,設(shè)置fetch-depth: 0check名稱不匹配CI顯示通過但PR仍被保護規(guī)則攔截校準保護規(guī)則中的check name規(guī)則過多導(dǎo)致走過場reviewer不再逐條確認精簡規(guī)則數(shù)量,每季度做規(guī)則體檢5. 把uReview用好的一些進階建議5.1 讓規(guī)則跟著項目走而不是跟著人走uReview最適合的落地場景,不是技術(shù)團隊搞一個“通用審查規(guī)范”,而是每個倉庫根據(jù)自身特點定義不同的規(guī)則。底層公共庫的審查關(guān)注點,跟業(yè)務(wù)服務(wù)是完全不一樣的;核心支付模塊的關(guān)注點,跟內(nèi)部管理系統(tǒng)也不一樣。把規(guī)則放到每個倉庫里,規(guī)則就跟著代碼一起走,代碼怎么演化,規(guī)則就怎么演化。我見過一些團隊想做一個“集團統(tǒng)一規(guī)則”然后把所有倉庫都套上同一套uReview配置,結(jié)果就是每個倉庫的PR評論區(qū)都出現(xiàn)大量跟自己無關(guān)的檢查項,沒多久大家就開始無視規(guī)則了。更合理的做法是,平臺團隊提供基礎(chǔ)規(guī)則模板,各業(yè)務(wù)倉庫按需裁剪和增補,保留真正相關(guān)的內(nèi)容。5.2 結(jié)合Code Owner實現(xiàn)分層審查uReview管的是“必須確認哪些問題”,但它不負責(zé)“誰來確認”。這個問題可以跟GitHub的CODEOWNERS機制結(jié)合起來用。CODEOWNERS負責(zé)規(guī)定哪些目錄的改動必須經(jīng)過哪些人審批,uReview負責(zé)審查記錄里必須確認哪些規(guī)則,兩者一動一靜,正好互補。比如一個改動同時碰了backend/和frontend/兩個目錄,通過CODEOWNERS,系統(tǒng)會自動把對應(yīng)兩個方向的負責(zé)人拉進來做review;而uReview的checklist會提醒后端負責(zé)人檢查接口兼容性和回滾方案,提醒前端負責(zé)人檢查API調(diào)用變更。這樣分工明確,不會出現(xiàn)“所有人都review了,但每個人都在看自己懂的那部分”的假象。5.3 從審查記錄里挖出流程改進點uReview產(chǎn)出的審查記錄本身也是一筆寶貴的數(shù)據(jù)資產(chǎn)。每一條規(guī)則的確認記錄,實際上反映了團隊審查過程的執(zhí)行情況。定期翻一翻這些記錄,你會發(fā)現(xiàn)很多有意思的信號:比如某條required規(guī)則總是被最后一個確認,可能說明它不是被遺漏,而是大家真的不關(guān)心;比如某個模塊的改動頻繁觸發(fā)“no test included”這條規(guī)則,可能說明相關(guān)開發(fā)者的測試基礎(chǔ)設(shè)施不順手。順著這些信號去優(yōu)化流程,比拍腦袋定制度要有效得多。我自己操作過的一個例子是:某條“變更是否包含灰度開關(guān)”的規(guī)則老是被跳過,后來發(fā)現(xiàn)是這個團隊的變更基本都是內(nèi)部工具,根本不走線上流量,這條規(guī)則對他們來說就是噪音。后來把它從required降級到informational,周會里扯皮的情況明顯少了。好用的工具不需要太多花哨功能,把流程里最煩瑣的那一環(huán)自動化掉,就已經(jīng)能帶來很大的效率提升。uReview的意義正在于此——它沒做什么高深的技術(shù),只是把“審查規(guī)則不落地”這個困擾無數(shù)團隊的經(jīng)典問題,用一個很直觀的機制解決掉了。如果你正在為代碼審查流程發(fā)愁,不妨先照這個思路把規(guī)則寫下來,再用工具推著你往前走一步。