How to Make Your Code Reviewer Fall in Love with You

Michael Lynch

如何讓你的程式碼審查者愛上你

談到程式碼審查時,大家的焦點往往放在審查者身上。但撰寫程式碼的開發者對於審查的重要性,並不亞於閱讀程式碼的人。關於如何為審查準備程式碼的指引少之又少,因此作者常常出於無知而搞砸了這個過程。

本文將介紹當你是作者時,參與程式碼審查的最佳實務。事實上,讀完這篇文章後,你送審程式碼的技巧將會精進到讓審查者真的會愛上你

但我不想讓我的審查者愛上我

他們就是會愛上你。接受吧。從來沒有人臨終時抱怨有太多人愛上自己。

為什麼要精進你的程式碼審查?

精進程式碼審查技巧能幫助你的審查者、你的團隊,最重要的是:你自己。

  • 更快學習:如果你妥善準備 changelist(變更清單),就能引導審查者將注意力放在有助於你成長的面向,而非無聊的風格違規。當你展現出對建設性批評的重視,審查者便會提供更好的回饋。

  • 讓他人變得更好:你的程式碼審查技巧為同事樹立了榜樣。有效的作者實務會感染你的隊友,當他們把程式碼送來給你審查時,你的工作也會更輕鬆。

  • 減少團隊衝突:程式碼審查是摩擦的常見來源。以審慎且認真的態度面對它們,能最大程度地減少爭執。

黃金法則:珍惜審查者的時間

這個建議聽起來顯而易見,但我經常看到作者把審查者當成專屬的品質保證技術人員。這些作者完全不花心思揪出自己的錯誤,也不為可審查性來設計 changelist。

你的隊友每天上班時,注意力都是有限的。如果他們分一部分給你,那就是無法用在自己工作上的時間。盡可能提升他們時間的價值,才是公平的做法。

當雙方彼此信任時,審查的品質會大幅提升。當審查者能確信你會認真看待他們的回饋時,就會投入更多心力。把審查者視為必須克服的障礙,會限制他們能為你帶來的價值。

技巧總覽

  1. 先自行審查你的程式碼
  2. 撰寫清晰的 changelist 說明
  3. 將簡單的事自動化
  4. 用程式碼本身回答問題
  5. 嚴格限縮變更範圍
  6. 區分功能性與非功能性變更
  7. 拆解過大的 changelist
  8. 優雅地回應批評
  9. 當審查者出錯時保持耐心
  10. 明確地傳達你的回應
  11. 巧妙地索取缺漏的資訊
  12. 平手時一律禮讓審查者
  13. 盡量縮短審查往返之間的延遲

1. 先自行審查你的程式碼

在把程式碼送給隊友之前,先自己讀一遍。別只是檢查錯誤——想像自己是第一次讀這段程式碼。有哪些地方可能會讓你感到困惑?

我發現撰寫程式碼和審查之間稍作休息很有幫助。許多人常在一天結束時急著送出變更,但那正是最容易忽略粗心錯誤的時候。等到隔天早上,用全新的眼光檢視 changelist,再交給隊友。

第一格:狗狗看著 changelist 問道:「哪個白癡寫的?」第二格:PR 標題為「將 cron 工作同步至月相週期」,說明寫著「我已新增此同步邏輯,以確保大自然與我們的 ETL pipeline 和諧一致。口述但未校閱」,署名正是第一格的那隻狗。第三格:狗狗一臉尷尬。

盡可能採用與審查者相同的環境。使用他們會看到的同一個 diff 檢視。在 diff 檢視中比在一般的原始碼編輯器中更容易發現愚蠢的錯誤。

別期待自己完美無缺。無可避免地,你有時會送出帶有忘記刪除的除錯程式碼或本應排除的額外檔案的 changelist。這些錯誤並非世界末日,但值得留意。注意自己犯錯的模式,並思考建立預防機制。如果這類錯誤太過頻繁,就等於向審查者傳達你不珍惜他們時間的訊號。

2. 撰寫清晰的 changelist 說明

在上一份工作中,我作為開發者導師計畫的一環,定期與一位資深工程師會面。在第一次會面前,他請我帶一份自己寫的設計文件。當我把文件遞給他時,我向他說明這個專案是什麼,以及它如何與團隊目標一致。我的導師皺起眉頭,直言道:「你剛剛告訴我的所有內容,都應該出現在設計文件的第一頁上。」

他是對的。我撰寫設計文件時,只想像了隊友會如何閱讀,卻忽略了其他讀者。除了直屬隊友之外,還有更廣泛的受眾,包括合作團隊、導師和升遷審查委員會。他們都應該能夠理解這份文件。自那次討論後,我總會思考如何為自己的工作提供脈絡說明。

你的 changelist 說明應該總結讀者所需的任何背景知識。你撰寫說明時心中或許已有特定的程式碼審查者人選,但他們不一定擁有你想像的脈絡。此外,其他隊友也可能需要閱讀這個 changelist,而未來的讀者在回顧變更歷史時,也應該能理解你的用意。

一份好的 changelist 說明會在高層次上解釋這項變更達成了什麼(what)以及你為何(why)要做這項變更。

若想更深入了解如何撰寫出色的 changelist 說明,請參閱我的文章,「How to Write Useful Commit Messages(《如何撰寫實用的 Commit 訊息》)」

3. 將簡單的事自動化

如果你依賴審查者來告訴你大括號放錯行,或你的變更導致自動化測試套件失敗,那你就是在浪費他們的時間。

狗狗打斷貓咪的工作,問道:「你可以幫我確認程式碼語法是否正確嗎?我本來想問編譯器,但我不想浪費它的時間。」

自動化測試應該是你團隊標準工作流程的一部分。審查應在所有自動化檢查於 continuous integration(持續整合) 環境中通過後才開始。

如果你的團隊執迷不悟,拒絕投資 continuous integration,請自行將這些檢查自動化。在你的開發環境中加入 Git pre-commit hooks、linters 和 formatters,以確保你的程式碼在每次 commit 時都能遵守適當的規範並保持預期的行為。

4. 用程式碼本身回答問題

這張圖有什麼問題?

mtlynch:我不太理解這個函式的用途。doggo:喔,這是為了防止呼叫者傳入了一個缺少 frombobulate 實作的 Frombobulator。

作者幫我理解了這個函式,但下一個閱讀它的人呢?難道他們要深入變更歷史,閱讀每一則程式碼審查討論嗎?更糟的是,作者直接走到我的座位前當面解釋,這不僅打斷了我的專注,也確保了其他人永遠無法取得這些資訊。

當審查者對程式碼的運作方式表示困惑時,解決方法不是只向那一個人解釋。你需要向所有人解釋。

狗狗:喂?貓咪:你六年前寫 bill.py 時,為什麼把 t 設為 6?狗狗:很高興你打來!因為營業稅是 6%。貓咪:原來如此!狗狗:這是傳達實作選擇的好方法。貓咪:微笑

回答他人問題的最佳方式是重構程式碼並消除困惑。你能否重新命名或重構邏輯,使其更清晰?程式碼註解是可接受的解法,但嚴格來說不如能自然自我說明的程式碼。

5. 嚴格限縮變更範圍

Scope creep(範圍蔓延)是程式碼審查中常見的反模式。一位開發者著手修復邏輯錯誤,卻在過程中注意到 UI 上的瑕疵。「既然都來了,」他心想,「就順手把那個也修了吧。」但這麼一來就把事情搞混了。審查者必須搞清楚哪些變更服務於目標 A,哪些服務於目標 B。

最好的 changelist 只做一件事。變更越小、越簡單,審查者就越容易在腦中掌握所有脈絡。將不相關的變更解耦,也能讓你在多位隊友之間平行進行審查,縮短變更的周轉時間。

6. 區分功能性與非功能性變更

限縮範圍的推論,就是要區分功能性與非功能性變更。

缺乏程式碼審查經驗的開發者經常違反這條規則。他們只做了兩行的變更,接著程式碼編輯器卻自動重排了整個檔案。開發者要不是沒意識到自己做了什麼,就是認為新的格式比較好。他們送出了一個被埋在數百行非功能性空白變更中的兩行功能性變更。

邏輯變更被空白變更所掩蓋的 changelist

你能在一片空白變更的雜訊中,找出被埋藏的功能性變更嗎?

混亂的 changelist 對審查者而言是極大的不尊重。只有空白的變更很容易審查。兩行的變更也很容易審查。但在空白變更的汪洋中迷失的兩行功能性變更,卻既繁瑣又令人抓狂。

開發者在重構時也常不當地混雜變更。我很樂見隊友重構程式碼,但我討厭他們在改變程式碼行為的同時進行重構。

邏輯變更被重構變更所掩蓋的 changelist

這個 changelist 只對行為做了一處變更,但重構變更卻將其掩蓋了。

如果一段程式碼既需要重構又需要行為變更,應該分成兩到三個 changelist 來進行:

  1. 新增測試以涵蓋現有行為(如果尚未有測試)。
  2. 在保持測試程式碼不變的情況下,重構正式程式碼。
  3. 在正式程式碼中變更行為,並更新測試以與之對應。

透過在步驟 2 中保持自動化測試不變,你向審查者證明了你的重構保留了原有行為。到了步驟 3 時,審查者就不必在重構變更中費力釐清行為變更,因為你已事先將它們解耦。

7. 拆解過大的 changelist

過大的 changelist 是 範圍蔓延醜陋的表親。假設一位開發者發現為了引入功能 X,必須修改現有程式庫 A 和 B 的語意。如果只是一小組變更,那還好,但太多這類蔓延的修改會讓 changelist 變得無比龐大。

changelist 的複雜度會隨著它觸及的程式碼行數呈指數成長。當我的變更超過 400 行正式程式碼時,我會在請求審查前尋找拆解的機會。

與其一次改變所有東西,你能否先變更相依項目,再於後續的 changelist 中加入新功能?你能否在先加入一半功能、下一個 changelist 再加入另一半的情況下,仍保持程式碼庫處於健全的狀態?

要拆解程式碼以找出一個能正常運作且易於理解的子集合,過程很繁瑣,但能帶來更好的回饋,也能減輕審查者的負擔。

8. 優雅地回應批評

毀掉程式碼審查最快的方式,就是把回饋當成針對個人。這很具挑戰性,因為許多開發者以自己的作品為傲,並將其視為自我的延伸。如果審查者以不得體的方式將回饋包裝得像人身攻擊,那就更難了。

身為作者,你最終能掌控自己對回饋的反應。請將審查者的意見視為關於程式碼的客觀討論,而非對你身為人的價值評判。防衛性地回應只會讓情況更糟。

我試著將所有意見都解讀為有益的學習。當審查者揪出我程式碼中令人尷尬的錯誤時,我的第一個本能是找藉口。相反地,我會克制自己,並讚美審查者的細心。

兩位開發者正在討論 changelist。doggo:這在 1900 年 1 月和 2 月其實會失效。mtlynch:哇,抓得好!

當審查者揪出你程式碼中細微的錯誤時,表達感謝。

令人驚訝的是,當審查者發現你程式碼中細微的缺陷時,其實是個好兆頭。這表示你把 changelist 包裝得很好。排除了格式不佳、命名混淆等顯而易見的問題後,審查者就能深入專注於邏輯與設計,從而提供更有價值的回饋。

9. 當審查者出錯時保持耐心

審查者有時就是會大錯特錯。正如你可能不小心寫出有缺陷的程式碼一樣,審查者也可能誤解正確的程式碼。

許多開發者對審查者的錯誤會以防衛心態回應。他們將那些甚至不正確的批評視為對自己程式碼的侮辱。

即使審查者弄錯了,那仍然是一個警訊。如果他們會誤讀,其他人是否也會犯同樣的錯誤?讀者是否必須付出異常高度的專注,才能確信某個錯誤並不存在?

兩位開發者在程式碼審查中爭論。mtlynch:這裡有緩衝區溢位,因為我們從未驗證 name 中配置的記憶體是否足夠容納 newNameLen 個字元。doggo:在我的程式碼裡?不可能!建構子呼叫了 PurchaseHats,而它又呼叫了 CheckWeather,如果緩衝區長度不正確,它就會回傳錯誤。拜託先把整個 20 萬行的程式碼庫完整讀過一遍,再來考慮我有可能犯錯的念頭。

當審查者犯錯時,抗拒證明他們是錯的誘惑。

尋找重構程式碼或加入註解的方法,讓程式碼更明顯地正確。如果困惑源於冷僻的語言特性,請改用非專家也能理解的機制來重寫程式碼。

10. 明確地傳達你的回應

我經常遇到這樣的情況:我給了對方意見,他們更新了程式碼以處理部分回饋,卻沒有留下任何回覆。現在,我們陷入了模糊的狀態。他們是漏看了我其他的意見,還是仍在處理中?如果我開始新一輪審查,可能會在尚未完成的 changelist 上浪費時間。如果我等待,可能會造成僵局,雙方都在期待對方先繼續。

在團隊中建立慣例,清楚地表明任何時刻是誰「拿著接力棒」。要不是作者正在修改,就是審查者正在撰寫回饋。絕不應該出現因為沒人知道該誰行動而導致流程停滯的情況。你可以透過在 changelist 層級留言來輕鬆達成這點,表明何時將主導權交還給對方。

作者留言「已更新!請再看一下。」的螢幕截圖

在 changelist 上留言,以明確傳達你何時將主導權交還給審查者。

對於每一則需要採取行動的意見,請明確回覆以確認你已處理。有些程式碼審查工具允許你將留言標記為已解決。否則,請遵循簡單的慣例,例如對每一則意見回覆「Done」。如果你不同意該意見,請有禮貌地說明你為何決定不採取行動。

Reviewable 介面顯示選項:discussing、satisfied、blocking 和 working。Satisfied 表示你認為自己已處理了審查者的意見。

ReviewableGerrit 這類程式碼審查工具提供了讓作者將特定意見標記為已解決的機制。

根據審查者付出的心力來調整你的回應。如果他們寫了詳細的意見來幫助你學習新知,別只是標記為完成。請深思熟慮地回應,以表達對他們付出的感謝。

11. 巧妙地索取缺漏的資訊

有時程式碼審查的意見留下了太多詮釋空間。當你收到像「這個函式令人困惑」這樣的留言時,你可能會想,「困惑」究竟是什麼意思?是函式太長嗎?是名稱不清楚嗎?還是需要更多文件說明?

很長一段時間,我都苦於如何在不顯得防衛的情況下釐清含糊的意見。我的直覺是問:「是哪裡令人困惑?」但這聽起來很不耐煩。

有一次,我無意間給隊友發了一則含糊的意見,而他的回應方式讓我覺得非常能化解敵意:

怎樣的修改會有所幫助?

我很喜歡這個回應,因為它展現了不帶防衛心且樂於接受批評的態度。每當審查者給我不明確的回饋時,我總會用「怎樣做會更有幫助?」這類說法來回應。

另一個有用的技巧是揣測審查者的意圖,並根據該假設主動修改程式碼。對於像「這令人困惑」這樣的意見,再仔細檢視一次你的程式碼。通常總有你可以改善清晰度的地方。一次修改就能向審查者傳達你樂於做出改變,即使那不是他們心中所想的改變。

12. 平手時一律禮讓審查者

在網球中,當你不確定對手的發球是否出界時,你會給予對方信任。同樣地,程式碼審查也應該有類似的期待。

一名球員若試圖在線審判決時做到一絲不苟地誠實,經常會讓一顆可能出界或事後才發現已出界的球繼續比賽。即便如此,用這種方式打球,比賽會精彩得多。

美國網球協會要求球員在進行線審判決時給予對手信任

關於程式碼的某些決定純屬個人品味。如果審查者認為你 8 行的函式拆成兩個 5 行的函式會更好,你們雙方都沒有客觀上的「正確」。哪個版本更好,只是觀點的問題。

當審查者提出建議,而你們雙方各有大致相當的證據支持自己的立場時,請禮讓審查者。在你們兩人之中,他們對於初次閱讀這段程式碼的感受,有著更好的視角。

13. 盡量縮短審查往返之間的延遲

幾個月前,一位使用者對我維護的一個 open-source(開源) 專案貢獻了一項小修改。我在幾小時內給予了回饋,但他們隨即消失了。幾天後我再次查看,仍然沒有任何回應。

六週後,這位神祕的開發者再度出現並提交了修改。雖然我感激他們的努力,但審查往返之間的延遲讓我的工作量加倍。我不僅得重讀他們的程式碼,還得重讀自己的回饋來恢復對討論的記憶。如果他們在一兩天內就跟進,我就不必做這些額外的工作。

審查者記憶與審查延遲的關係圖,顯示當審查往返之間出現長時間延遲時所浪費的心力。

六週的停頓算是極端案例,但我在隊友之間經常看到漫長且不必要的延遲。有人送出 changelist 進行審查,收到回饋後,卻因為被另一項任務分心而將其擱置一週。

除了因恢復脈絡而損失的時間外,未完成的 changelist 也會增加複雜度。它們讓所有人更難追蹤哪些已經合併、哪些仍在進行中。隨著部分完成的 changelist 越多,合併衝突也越多,而沒有人喜歡處理那些。

一旦你送出程式碼,推動審查直至完成應該是你的最優先要務。你這端的延遲會浪費審查者的時間,並增加整個團隊的複雜度。

結論

當你準備下一個要送審的 changelist 時,請思考你能掌控的因素,並運用它們來引導審查更具成效。在參與審查時,留意那些會停滯進度或浪費心力的模式。

請記住黃金法則:珍惜審查者的時間。當你讓審查者能專注於程式碼中有趣的部分時,他們就能產生高品質的回饋。如果你要求他們去解開糾纏的程式碼或糾正簡單的錯誤,雙方都會受害。

最後,請用心溝通。簡單的誤解或未經思考的留言,極其容易讓審查脫軌。在評論他人作品時情緒往往高漲,因此要留意那些可能讓審查者感到被攻擊或不受尊重的陷阱。

恭喜!如果你已經讀到這裡,你現在已經是專業的被審查者了。你的審查者很可能已經愛上你,所以請好好對待他們。

延伸閱讀


插圖由 Loraine Yow(蘿蘭·尤)繪製。由 Samantha Mason(莎曼珊·梅森) 編輯。

原文由 Michael Lynch 發布

本文章由 muse-spark-1.2-contributor 進行翻譯