如何像人一樣做程式碼審查（下）
這是我關於如何在程式碼審查中有效溝通並避免陷阱的文章的下半部。在這裡，我將聚焦於如何讓你的程式碼審查順利收尾，同時避免難看的衝突的技巧。
我在上篇中已奠定基礎，因此我建議從那裡開始閱讀。如果你等不及了，這裡是精簡版：一位優秀的程式碼審查者不僅會找出錯誤，還會提供用心的回饋，幫助隊友成長。
我經歷過最糟的一次程式碼審查
我人生中最糟的一次程式碼審查，對象是一位我姑且稱為 Mallory（瑪洛莉）的前隊友。她比我早好幾年進入公司，但直到最近才轉調到我的團隊。
審查過程
當瑪洛莉傳來她的第一個變更清單請我審查時，程式碼有點粗糙。她以前從未寫過 Python，而且是在我負責維護的一個笨重老舊系統之上進行開發。
我盡責地記錄下所有發現的問題，總共有 59 個。根據我讀過的審查相關文獻，我做得非常出色。我找到了這麼多錯誤，因此，我一定是個優秀的審查者。
幾天後，瑪洛莉傳來更新後的變更清單以及她對我意見的回覆。她已經修正了簡單的問題：錯字、變數重新命名等等。但她拒絕處理更高層次的問題，例如她的程式碼對格式錯誤的輸入會有未定義行為，或是其中一個函式將控制流程結構巢狀了六層之深。相反地，她輕描淡寫地解釋說，這些問題不值得花工程時間去修正。
憤怒又沮喪的我，又送出了一輪意見。我的語氣表面上專業，卻逐漸帶有被動攻擊的意味。「你可以解釋一下為什麼我們會想要對格式錯誤的輸入產生未定義行為嗎？」正如你所料，瑪洛莉的回覆變得更加固執。
痛苦的循環
一週後的星期二，瑪洛莉和我仍在為同一個審查來回爭辯。前一晚我才剛傳出最新一輪的意見。我故意等到她下班離開後才送出，因為我不想和她待在同一個空間裡看她讀到這些意見。
整個早上，我的胃都沉甸甸的，對下一輪審查感到焦慮不安。我吃完午餐回來，看到瑪洛莉不在座位上，但已經傳來了新的修改。我想她也不想待在現場看我讀她的回覆。
隨著她的每一則回覆讓我愈來愈憤怒，我的心臟開始在胸口狂跳。我立刻猛敲鍵盤逐一反駁，指出她既沒有照我的建議修改，也沒有提出足以讓我核准的理由。
我們每天重複這樣的循環，持續了三週。程式碼幾乎沒有什麼變化。
介入
幸好，我們最資深的隊友 Bob（鮑伯）打破了這個循環。他結束長假回來，驚訝地發現我們正火藥味十足地來回丟著程式碼審查意見。他立刻認清狀況的本質：這是一場僵局。他提出由他接手審查，我們兩人都同意了。
鮑伯一開始便請瑪洛莉建立新的變更清單，將兩個我們其實從未真正爭執過的小型函式庫拆分出來，每個大約 30 到 50 行。瑪洛莉照做後，鮑伯立刻就核准了。
接著，鮑伯回到主要的變更清單，此時已精簡到約 200 行程式碼。他提出了幾個小建議，瑪洛莉也都處理了。然後，他就核准了這個變更清單。
鮑伯的整個審查在兩天內就完成了。
溝通很重要
你可能已經推斷出，這場衝突其實並非真正關於程式碼。程式碼確實有正當的問題，但這些問題對於能夠有效溝通的隊友來說，顯然是可以解決的。
這是一次不愉快的經驗，但回過頭來看，我很慶幸有過這樣的經歷。它促使我重新評估自己進行審查的方式，並找出需要改進的地方。
接下來，我將分享一些能降低你遭遇類似不愉快結果風險的技巧。稍後我會再回到瑪洛莉的例子，解釋為什麼我原本的做法是本末倒置，以及為什麼鮑伯的做法是那樣不著痕跡卻高明。
技巧
試著將程式碼提升一到兩個等級
雖然你的隊友在理論上或許想探索每個改進程式碼的機會，但他們的耐心是有限的。如果你一輪又一輪地扣住核准，只因為你不斷想到新的、絕妙的方法來幫他們打磨變更清單，他們很快就會感到沮喪。
我私下會用等級來評價程式碼，從 A 到 F。當我收到一個起始分數是 D 的變更清單時，我會試著協助作者將它提升到 C 或 B-。不完美，但已經足夠好。
理論上，要把 D 提升到 A+ 是可能的，但可能需要多達八輪以上的審查。到最後，作者會討厭你，再也不想把程式碼送給你審。
你可能會想：「如果我接受 C 級的程式碼，難道最後不會得到一個 C 級的程式碼庫嗎？」幸好，答案是不會。我發現，當我幫助隊友從 D 進步到 C 之後，他們下一次傳給我的變更清單起始分數就會是 C。幾個月內，他們傳來的審查起點就變成 B，並在審查結束時成為 A。
F 是保留給功能上有錯誤，或是複雜到讓你對其正確性毫無信心的程式碼。唯一應該扣住核准的理由，是程式碼在經過幾輪審查後仍然是 F。請參閱下方的僵局一節。
限制對重複模式的回饋
當你發現作者的好幾個錯誤都屬於同一種模式時，不要把每一個實例都標出來。你不會想花時間把同樣的意見寫 25 次，作者當然也不想讀 25 則重複的意見。
指出同一模式中的兩、三個個別實例是沒問題的。超過這個數量，就直接請作者修正整個模式，而不是逐一修正每個出現的地方。
尊重審查的範圍
我經常看到一種反模式，審查者在變更清單中發現附近的程式碼有問題，便要求作者一併修正。作者照做之後，審查者通常會發現程式碼雖然變好了，卻不一致，因此還需要再做一些小修改。然後又要再改一些。就這樣沒完沒了，直到一個原本範圍狹小的變更清單膨脹成包含大量不相關異動的清單。
如果一隻飢餓的小老鼠出現在你家門口，你可能會想給牠一塊餅乾。而如果你給牠一塊餅乾，牠就會想要一杯牛奶。牠會想照鏡子確認自己沒有留下牛奶鬍，然後牠就會想要一把剪刀來幫自己修剪一下……
-Laura Joffe Numeroff（蘿拉·喬菲·努梅羅夫），If You Give a Mouse a Cookie（《如果給老鼠一塊餅乾》）
經驗法則是：如果變更清單沒有碰到那一行，它就不在審查範圍內。
以下是一個範例：
即使程式碼庫中那個魔術數字和荒謬的變數名稱會讓你整夜輾轉難眠，它還是不在範圍內。即使作者就是寫下附近那幾行的人，也依然不在範圍內。如果情況真的糟到離譜，請另行提交錯誤單或自己送出修正，但不要在這次審查中強加到作者身上。
例外情況是當變更清單在未實際碰觸周圍程式碼的情況下，卻影響了它，例如：
在這種情況下，請指出作者需要將函式從 ValidateAndSerialize 重新命名為 Serialize。他們雖然沒有碰到包含函式簽名的那一行，但仍導致它變得不正確。
如果我沒有太多意見，但注意到範圍外有個輕鬆就能修正的問題，我會稍微打破這個規則。在這種情況下，我會明確表示作者可以自行決定是否忽略這則意見。
尋找機會拆分大型審查
如果你收到超過約 400 行程式碼的變更清單，請鼓勵作者將其拆成更小的部分。超出越多，就越要強力要求拆分。我個人拒絕審查任何超過 1,000 行的變更清單。
作者可能會抱怨拆分變更清單很煩瑣。你可以透過找出合乎邏輯的拆分邊界來減輕他們的負擔。最簡單的情況是變更清單獨立地更動了多個檔案，這時他們只要把變更清單按檔案拆成較小的集合即可。在較困難的情況下，找出位於最底層抽象層級的函式或類別。請作者將這些移到另一個變更清單，然後在第一個變更清單合併後，再回頭處理其餘的程式碼。
當程式碼品質低落時，務必強烈要求拆分。審查糟糕程式碼的難度會隨著大小呈指數成長。與其審查一個 600 行的糟糕巨作，不如分開審查兩個 300 行、寫得馬虎的變更清單，效果會好得多。
給予真誠的讚美
大多數審查者只關注程式碼哪裡有錯，但審查其實是強化正面行為的寶貴機會。
舉例來說，想像你正在審查一位不太會寫文件的作者的程式碼，卻看到一則清晰簡潔的函式註解。請讓他們知道他們做得很棒。如果你能在他們做對時給予肯定，而不是只等他們出錯時才扣分，他們會進步得更快。
你不需要懷抱特定目的才給予讚美。任何時候我在變更清單中看到讓我驚艷的地方，我都會告訴作者：
- 「我以前不知道這個 API。這真的很實用！」
- 「這是個優雅的解法。我從來沒想過可以這樣做。」
- 「把這個函式拆開真是個好主意。現在簡單多了。」
如果作者是資淺的開發者或剛加入團隊，他們在審查過程中很可能會感到緊張或防衛。真誠的讚美能緩解這種緊張感，讓他們知道你是支持他們的隊友，而不是殘酷的守門人。
當剩餘的修正微不足道時就給予核准
有些審查者誤以為應該扣住核准，直到親眼看到每一則意見都被修正。這會增加不必要的審查輪次，浪費作者與審查者雙方的時間。
在下列任一情況成立時，就給予核准：
- 你已經沒有其他意見。
- 剩餘的意見都是瑣碎問題。
- 例如：重新命名變數、修正錯字
- 剩餘的意見屬於可選擇性的建議。
- 請明確標示這些是可選的，以免隊友誤以為你的核准取決於這些建議。
我曾看過審查者因為作者在程式碼註解結尾漏掉一個句號就扣住核准。請不要這樣做。這會讓作者覺得你認為他們連加上簡單標點符號的能力都沒有，需要人監督才行。
在仍有未處理意見的情況下給予核准，確實有些風險。我估計大約有 5% 的機率，作者會誤解最後一輪的意見或完全漏掉。為了降低這個風險，我會在核准後簡單檢查作者的後續修改。在少數溝通不良的情況下，我要麼再與作者聯繫，要麼自己建立一個變更清單來修正。為 5% 的情況增加少許工作量，總比為另外 95% 增加不必要的負擔和延誤來得好。
主動處理僵局
程式碼審查最糟的結果就是僵局：你拒絕在未進一步修改的情況下簽核變更清單，但作者也拒絕做出修改。
以下是一些顯示你正走向僵局的徵兆：
- 討論的語氣愈來愈緊張或帶有敵意。
- 你每輪審查提出的意見數量沒有呈下降趨勢。
- 你的意見中有異常多數遭到反彈。
好好談一談
面對面開會或透過視訊對談。文字溝通很容易讓人忘記對話另一端有個真實的人。你會很容易想像隊友是出於固執或無能才那樣回應。一場會議將能打破這種魔咒，對你和作者都是如此。
考慮進行設計審查
一場充滿爭議的程式碼審查，可能顯示流程更早階段就存在弱點。你們爭論的事情，是否本應在設計審查時就涵蓋？到底有沒有做過設計審查？
如果分歧的根源可追溯到高層次的設計選擇，就應該讓更廣泛的團隊參與，而不是留給剛好參與程式碼審查的兩個人來決定。與作者討論以設計審查的形式，將討論開放給團隊其他成員。
讓步或向上呈報
你和隊友在僵局中僵持得越久，對彼此關係的傷害就越大。如果其他替代方案都無法讓你們脫困，你的選擇就是讓步或向上呈報。
衡量一下直接核准這些修改的代價。如果你隨意接受低品質的程式碼，就無法打造出高品質的軟體；但如果你和隊友爭得如此激烈以至於無法再合作，也同樣無法達到高品質。如果核准這個變更清單，實際上會有多糟？它是可能摧毀關鍵資料的程式碼嗎？還是只是一個背景處理程序，最糟的情況不過是工作失敗、需要開發者去除錯？如果比較接近後者，請考慮乾脆讓步，以便與隊友維持良好的合作關係。
如果讓步並非選項，請與作者討論將討論向上呈報給團隊的主管或技術負責人。主動提出改由另一位審查者接手。如果呈報後的決定對你不利，就接受決定並繼續前進。繼續爭執只會讓糟糕的局面拖得更久，並讓你顯得不專業。
從僵局中復原
混亂的審查爭論往往與其說是關於程式碼，不如說是關於相關人員之間的關係。如果你已經陷入僵局或接近僵局，若不處理根本的衝突，這種模式就會重演。
- 與你的主管討論情況。
- 如果團隊中有衝突，你的主管應該要知道。也許作者本身就很難合作。也許你正在以自己未察覺的方式助長了這個局面。一位好的主管會幫助你們雙方處理這些問題。
- 暫時別和對方互相審查。
- 如果可能，在事情冷卻下來的幾週內，避免互相發送程式碼審查。
- 學習衝突解決。
- 我發現 Crucial Conversations（《關鍵對話》）這本書很有幫助。它的建議聽起來或許像是常識，但在你並未身處爭執當下時，分析自己處理衝突的方式具有極大的價值。
回顧我最糟的那次程式碼審查
還記得與瑪洛莉的那次程式碼審查嗎？為什麼我的審查會變成在被動攻擊的泥沼中苦撐三週，而鮑伯的卻在兩天內輕鬆完成？
我哪裡做錯了
這是瑪洛莉在團隊中的第一次審查。我沒有考慮到她可能會感到被評判或產生防衛心。我應該一開始只提出高層次的意見，這樣她才不會因大量的意見而感到被突襲。
我應該多做一些來表明我的工作不是阻礙她，而是幫助她推進。我本可以提供程式碼範例，或指出她變更清單中的優點。
我讓自我影響了審查。過去一年我一直在悉心修復這個老舊系統。突然出現一個新人對它隨意擺弄，卻連認真看待我的顧慮都懶得做？我將此視為一種冒犯，但這種態度適得其反。我應該保持我在所有審查中都努力維持的客觀心態。
最後，我讓僵局拖延得太久。幾輪之後，我就應該清楚我們並未取得有意義的進展。我應該做出果斷的改變，例如當面開會以處理更深層的衝突，或向上呈報給主管。
鮑伯做對了什麼
鮑伯一開始拆分審查的做法非常有效。回想一下，那個已經痛苦卡關三週的審查，突然間有兩段程式碼被合併了。這讓瑪洛莉和鮑伯都感到愉快，因為它建立了前進的動能。剩下的部分仍有問題，但它已經變成一個更小、更容易管理的變更清單。
鮑伯沒有試圖把審查逼到完美。他很可能也認出了那些我大聲疾呼的問題，但他意識到瑪洛莉還會在團隊待上一段時間。他在短期內的彈性，讓他能夠在長期上幫助瑪洛莉提升品質。
結論
在我發表這篇文章的上半部之後，有幾位讀者對我推薦的溝通方式提出異議。有些人覺得那樣居高臨下。另一些人則擔心那樣太過間接，有造成誤解的風險。
這些回饋是合理且可預期的。一個人可能會覺得簡短的審查意見很唐突或粗魯，另一個人則可能認為同樣的意見簡潔而有效率。
在審查程式碼時，你會做出許多選擇：要聚焦什麼、如何措辭回饋、何時給予核准。重要的是，你選擇的並非一定是我的選項，而是要意識到確實存在不同的選項。
沒有人能給你一份完美審查的食譜。最有效的技巧將取決於程式碼作者的個性、你與他們的關係，以及你們團隊的文化。透過批判性地思考程式碼審查的結果，來磨練你的方法。當你遇到緊張局面時，退一步評估它為何發生。留意你審查的品質。如果你覺得無法將程式碼提升到你的品質標準，請思考審查流程中的哪些環節阻礙了你，以及你該如何解決。
祝你好運，願你的程式碼審查都能像人一樣充滿人性。
延伸閱讀
- 〈如何讓你的程式碼審查者愛上你〉是本文的姊妹篇。它描述了當你是作者而非審查者時，如何改善程式碼審查。
- 由 PuTTY SSH 客戶端的作者 Simon Tatham（賽門·塔森）所寫的〈程式碼審查反模式〉，涵蓋了一份作為審查者應避免的陷阱的實用清單。
本文由 Samantha Mason（莎曼珊·梅森） 編輯。插圖由 Loraine Yow（蘿芮恩·尤）繪製。感謝 @global4g 對本文早期草稿提供寶貴的回饋。