How to Do Code Reviews Like a Human (Part Two)

2017-11-09

Michael Lynch

如何像人一樣做 Code Review（下篇）

原文由 Michael Lynch 于 2017-11-09 發布，訂閱此部落格

這是關於如何在 Code Review 中有效溝通、避免陷阱的文章的下半部。在這一篇中，我將聚焦於如何讓 Code Review 順利收尾，同時避免難堪衝突的技巧。

我已經在上篇打下了基礎，所以建議從那裡開始看起。如果你沒耐心，這裡是精簡版：好的審查者不只是找出 bug，更會提供用心的回饋，幫助隊友成長。

我經歷過最糟的一次 Code Review

我人生中最糟的一次 Code Review，對象是我一位姑且稱之為 Mallory 的前同事。她比我早好幾年進公司，但才剛轉調到我們團隊不久。

審查過程

當 Mallory 把她的第一個 changelist 送來給我審查時，程式碼有點粗糙。她以前從沒寫過 Python，而她是在我負責維護的一套笨重老舊系統上做開發。

我盡責地記下了所有發現的問題，總共 59 項。按照我讀過的審查相關文獻來看，我表現得非常出色。我找出了超多錯誤，所以我一定是個好審查者。

幾天後，Mallory 寄來更新後的 changelist 以及她對我意見的回覆。她修掉了簡單的問題：錯字、變數重新命名等等。但她拒絕處理更深層的問題，例如她的程式碼在輸入格式錯誤時會產生未定義行為，或是其中一個函式裡的控制流程竟巢狀了六層之深。相反地，她輕描淡寫地解釋說，這些問題不值得花工程時間去修。

憤怒又挫折的我，又寄出了一輪意見。我的語氣表面上專業，卻逐漸帶點陰陽怪氣。「可以請你解釋一下為什麼我們會想要讓格式錯誤的輸入產生未定義行為嗎？」如你所料，Mallory 的回覆變得更加強硬。

苦澀的循環

一週後的星期二，我和 Mallory 還在為同一個審查來回拉鋸。前一晚我才寄出最新一輪的意見。我故意等到她下班離開後才寄出，因為我不想跟她待在同一個空間，看她讀到那些意見。

整個早上，我胃裡都像壓著一塊大石頭，惴惴不安地等待著下一輪審查。我吃完午餐回來，看到 Mallory 不在座位上，但她已經寄來了新的修改。我猜她也不想留在現場看我讀她的回覆。

隨著她的每一則回覆，我愈看愈火大，心跳也愈來愈快。我立刻猛敲鍵盤寫下反駁，指出她既沒有照我的建議修改，也沒有提出足以讓我放行的理由。

這樣的戲碼每天重演，持續了三個星期。而程式碼幾乎沒有什麼進展。

介入

幸好，我們團隊最資深的同事 Bob 打破了這個循環。他休完長假回來，驚訝地發現我們正針鋒相對地互丟審查意見。他立刻看穿了現狀的本質：僵局。他主動提出要接手這個審查，我們兩人都同意了。

Bob 一開始先請 Mallory 建立新的 changelist，把其中兩個我們其實沒什麼爭議的小函式庫獨立拆出來，每個大約 30 到 50 行。Mallory 照做後，Bob 馬上就核准了。

接著，Bob 回到主 changelist，此時已經精簡到大約 200 行程式碼。他提了幾個小建議，Mallory 也都處理了。然後，他就核准了這個 changelist。

Bob 的整個審查只花了兩天就完成了。

溝通才是關鍵

你可能已經看出，這場衝突其實跟程式碼本身無關。程式碼確實有正當的問題，但在能有效溝通的隊友之間，這些問題明明是可以解決的。

那是一次很不愉快的經驗，但回頭看，我很慶幸有過那次經歷。它讓我重新檢視自己做審查的方式，並找出需要改進的地方。

接下來，我會分享幾個能降低你陷入類似困境風險的技巧。稍後我會再回到 Mallory 的例子，說明為什麼我原本的做法是本末倒置，而 Bob 的做法又是何以高明得不著痕跡。

技巧

試著把程式碼提升一到兩個等級

雖然你的隊友理論上可能會想探索所有改進程式碼的機會，但他們的耐心是有限的。如果你一輪又一輪地扣住核准，只因為你又想到了新的、絕妙的方法來打磨他的 changelist，他們很快就會感到挫折。

我私底下會用學校成績的等級來幫程式碼打分數，從 A 到 F。當我收到一個一開始只有 D 的 changelist 時，我會試著幫助作者把它提升到 C 或 B-。不完美，但已經夠好了。

理論上，當然有可能把 D 拉到 A+，但那大概需要八輪以上的審查。到最後，作者會恨死你，再也不想把程式碼送給你審。

你可能會想：「如果我接受 C 等級的程式碼，那整個程式碼庫不就變成 C 等級了嗎？」幸好不會。我發現當我幫隊友從 D 提升到 C 之後，他們下一次寄來的 changelist 就會直接從 C 起跳。幾個月內，他們寄來的審查就已經是 B 起跳，審完後則變成了 A。

F 是保留給功能上就是錯的、或是複雜到讓你無法對其正確性抱有信心的程式碼。只有在經過幾輪審查後，程式碼仍停留在 F 時，你才應該扣住不核准。請見下方關於僵局的段落。

針對重複出現的模式，點到為止

當你發現作者的好幾個錯誤都屬於同一種模式時，不要每一個都標出來。你不會想花時間把同一則意見寫 25 遍，作者也絕對不想讀到 25 則重複的意見。

點出其中兩、三個例子是沒問題的。超過這個數量，就直接請作者修正這個模式本身，而不是逐一修正每個出現的地方。

尊重審查的範圍

我常看到一種反模式：審查者在 changelist 附近的程式碼中發現問題，就要求作者順手修掉。作者照做後，審查者又發現程式碼雖然變好了，卻變得不一致，所以還需要再做幾個小修改。然後又要再改幾個。就這樣沒完沒了，直到一個原本範圍很小的 changelist 膨脹成一堆不相關的異動。

如果一隻飢餓的小老鼠出現在你家門口，你可能會想給他一塊餅乾。而如果你給了他一塊餅乾，他就會跟你要一杯牛奶。他會想照照鏡子，確定自己沒有牛奶鬍，然後他又會跟你要一把剪刀，想幫自己修剪一下……
-蘿拉·喬菲·紐梅洛夫，《如果給老鼠一塊餅乾》

經驗法則是：如果 changelist 沒有碰到那一行，那就是超出範圍。

舉個例子：

即使你的程式碼庫裡那個魔術數字和荒謬的變數名稱會讓你整夜輾轉難眠、揮之不去，它還是超出範圍。即使作者就是寫下附近那幾行的人，也依然超出範圍。如果真的糟到不行，就另外開一個 bug 或自己送一個修正，但不要在這次審查中硬塞給作者。

例外是當 changelist 雖然沒有實際碰到周圍的程式碼，卻影響到了它，例如：

在這種情況下，你就要指出作者需要把函式從 ValidateAndSerialize 重新命名為 Serialize。他們雖然沒有碰到函式簽名的那一行，卻讓它變得不正確了。

如果我沒有太多意見，卻注意到範圍外有個輕鬆就能修掉的問題，我會稍微放寬這個原則。在這種情況下，我會明確表示作者可以自行決定要不要理會這則意見。

尋找機會把大型審查拆開

如果你收到超過約 400 行程式碼的 changelist，就鼓勵作者把它拆成更小的部分。超過越多，就越要強硬地要求拆分。我個人是拒絕審查任何超過 1,000 行的 changelist。

作者可能會抱怨拆分 changelist 很麻煩。幫他們減輕負擔的方式，就是幫他們找出邏輯上的切割點。最簡單的情況是 changelist 獨立地動到了多個檔案，這時只要把 changelist 按檔案拆成幾個較小的集合就行。在比較棘手的情況下，就去找最底層抽象的函式或類別，請作者把這些部分移到另一個獨立的 changelist，等第一個 changelist 合併後，再回頭處理剩下的程式碼。

當程式碼品質很差時，更要強烈要求拆分。審查糟糕程式碼的難度會隨著大小呈指數成長。與其審一個 600 行的災難，倒不如審兩個 300 行、寫得有點隨便的 changelist 來得輕鬆。

給予真誠的讚美

大多數審查者只關注程式碼哪裡有問題，但審查其實是強化正面行為的寶貴機會。

舉例來說，假設你正在幫一位不太會寫文件的作者做審查，卻看到一段清晰簡潔的函式註解，就告訴他這段寫得很好。如果你在他們做對時給予肯定，而不是只等著他們出錯時才扣分，他們會進步得更快。

你不需要抱著特定目的才給予讚美。任何時候我在 changelist 中看到讓我眼睛一亮的地方，我都會告訴作者：

「我之前不知道有這個 API，真的很實用！」
「這是個很優雅的解法，我完全沒想到。」
「把這個函式拆開真是個好主意，現在簡單多了。」

如果作者是資淺的開發者或是剛加入團隊的新人，在審查時很可能會感到緊張或防衛。真誠的讚美能緩解這種緊張感，讓對方知道你是支持他的隊友，而不是冷酷的守門人。

當剩下的是瑣碎修正時，就予以核准

有些審查者誤以為要等到親眼看到每一則意見都被修正後才能核准。這會徒增不必要的審查往返，浪費作者和審查者雙方的時間。

在以下任何一種情況成立時，就予以核准：

你已經沒有其他意見了。
剩下的意見都是瑣碎的問題。
- 例如：重新命名變數、修正錯字
剩下的意見只是選擇性的建議。
- 請明確標示這些是選擇性的，這樣隊友才不會以為你的核准取決於這些建議。

我看過有審查者因為作者在程式碼註解結尾少了一個句號就扣住不核准。拜託不要這樣做。這等於在告訴作者，你覺得他連加個標點符號都需要人監督才做得到。

在還有未處理意見的情況下就核准，確實有一定風險。我估計大約有 5% 的機率，作者會誤解最後一輪的意見，或是完全漏掉。為了降低這個風險，我會在核准後簡單檢查作者後續的修改。在少數溝通不良的情況下，我會再跟作者確認，或是自己開一個 changelist 來修正。與其讓另外 95% 的情況都增加不必要的負擔和延遲，不如只在 5% 的情況多花一點點功夫。

主動處理僵局

Code Review 最糟的結果就是僵局：你堅持不核准，除非對方再做修改，但作者也堅持不改。

以下幾個跡象表示你正走向僵局：

討論的語氣愈來愈緊張或充滿敵意。
每一輪審查的意見數量沒有逐漸減少。
你的意見遭到異常大量的反彈。

好好談一談

約個時間面對面或用視訊聊聊。純文字的溝通很容易讓人忘記對話的另一端是個活生生的人，也很容易讓你誤以為隊友是出於固執或無能才那樣回應。一場會面能為你和作者打破這種錯覺。

考慮做設計審查

一場充滿爭議的 Code Review，可能代表流程更早的階段就出了問題。你們爭論的事情，是不是本來就該在設計審查時就討論完？到底有沒有做過設計審查？

如果分歧的根源可以追溯到高層次的設計決策，那就應該讓更廣泛的團隊一起參與，而不是只丟給剛好在做 Code Review 的兩個人去決定。跟作者談談，把討論擴大到整個團隊，以設計審查的形式來處理。

讓步或升級

你和隊友在僵局中糾纏得愈久，對彼此關係的傷害就愈大。如果其他方法都無法讓你們脫困，你的選擇就只剩下讓步或升級處理。

衡量一下直接核准的代價。如果隨便接受低品質的程式碼，你當然無法打造出高品質的軟體，但如果你和隊友吵到水火不容、無法再合作，也同樣無法達到高品質。如果核准這個 changelist，真的會有多糟？是那種可能會毀掉關鍵資料的程式碼嗎？還是只是一個背景處理程序，最糟的情況不過是任務失敗、需要開發者去除錯？如果比較接近後者，那就考慮直接讓步，這樣才能和隊友維持良好的合作關係。

如果讓步不是選項，就跟作者商量把討論升級，交給團隊的主管或技術負責人來定奪。也可以主動提出換一位審查者。如果升級後的決定對你不利，就接受它然後往前走。繼續糾纏只會讓糟糕的局面拖得更久，也會讓你顯得不專業。

從僵局中復原

紛亂的審查爭執，往往與其說是關於程式碼，不如說是關於參與者之間的關係。如果你已經陷入僵局或接近僵局，若不處理根本的衝突，這種模式就會一再重演。

跟你的主管談談這個情況。
- 如果團隊中有衝突，你的主管應該要知道。也許作者本來就很難合作，也許你在不自覺中也加劇了局面。好的主管會幫助你們雙方處理這些問題。
暫時別再互相審查。
- 如果可行，幾個星期內盡量避免互送 Code Review，等彼此都冷靜下來。
學習衝突處理。
- 我覺得 Crucial Conversations 這本書很有幫助。它的建議聽起來也許像是常識，但在你沒有身處爭執當下時，仔細分析自己處理衝突的方式，是非常有價值的。

回顧我最糟的那次 Code Review

還記得和 Mallory 的那次 Code Review 嗎？為什麼我的審查會變成在陰陽怪氣的泥沼中苦撐三週，而 Bob 卻能在兩天內輕鬆搞定？

我哪裡做錯了

那是 Mallory 在這個團隊的第一次審查。我沒有考慮到她可能會感到被評判或產生防衛心。我應該要一開始只提高層次的意見，才不會讓她被大量的意見給突襲。

我應該要更努力地表明，我的任務不是阻礙她的工作，而是幫助她推進。我本可以提供程式碼範例，或是點出她 changelist 中的優點。

我讓自尊心影響了審查。過去一年我費心把這套老系統搶救回來，突然有個新人跑來亂動它，卻不把我的顧慮當一回事？我把這當成一種冒犯，但這種心態只會適得其反。我應該要保持我一向試圖在所有審查中維持的客觀心態。

最後，我讓僵局拖得太久了。經過幾輪之後，我就應該要看出我們並沒有取得實質進展。我應該要採取更果斷的行動，例如約個面對面的會談來處理更深層的衝突，或是升級給主管處理。

Bob 做對了什麼

Bob 第一招把審查拆開就非常有效。回想一下，那個已經痛苦卡關三週的審查，突然就有兩塊程式碼被合併了。這讓 Mallory 和 Bob 都感覺很好，因為它建立了前進的動能。剩下的部分雖然仍有問題，但已經變成一個更小、更容易處理的 changelist。

Bob 沒有試圖把審查逼到完美。他很可能也看到了我當初大呼小叫的那些問題，但他明白 Mallory 會在團隊待上一段時間。他在短期內的彈性，讓他能在長期上更好地幫助 Mallory 提升品質。

結論

在我發表這篇文章的上半部之後，有幾位讀者對我建議的溝通方式持不同意見。有些人覺得那樣有點高高在上、帶有施捨感。也有人擔心那樣太過迂迴，反而有造成誤解的風險。

這些回饋是合理且可預期的。同樣一句簡短的審查意見，有人可能覺得粗魯無禮，另一個人卻可能認為是簡潔高效。

在審查程式碼時，你會面臨許多選擇：要聚焦什麼、如何措辭、什麼時候核准。你不需要選擇我的做法，重要的是要意識到你是有選擇的。

沒有人能給你一份完美審查的食譜。最有效的技巧會取決於程式碼作者的個性、你與他的關係，以及你們團隊的文化。透過批判性地思考每次 Code Review 的結果，來磨練你的方法。當你遇到緊張氣氛時，退一步評估它是怎麼發生的。留意你審查的品質。如果你覺得無法把程式碼提升到符合你的品質標準，就想想審查流程中有哪些環節阻礙了你，以及你該如何解決。

祝你好運，願你的 Code Review 都充滿人味。