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

Michael Lynch

如何像真人一樣進行程式碼審查(上篇)

最近,我一直在閱讀關於程式碼審查最佳實務的文章。我注意到這些文章只專注於找出錯誤,幾乎排除了審查的其他所有面向。以具建設性且專業的方式溝通你發現的問題?不重要!只要找出所有的錯誤,剩下的自然會迎刃而解。

所以我靈光一現:如果這套方法對程式碼有用,為什麼不能用在愛情上呢?因此,我要宣布推出一本幫助開發者改善感情生活的新電子書:

電子書封面

我的這本革命性電子書將教你經過驗證的技巧,幫助你最大化找出伴侶缺點的數量。這本電子書不會涵蓋:

  • 以同理心與理解與伴侶溝通問題。
  • 幫助伴侶改善他們的弱點。

根據我對程式碼審查文獻的閱讀,感情關係中的這些部分是顯而易見不值得討論的。

你覺得這聽起來像是一本好電子書嗎?我想你剛剛一定大喊了「不不不不不!」

那麼,為什麼我們談論程式碼審查時就是這樣呢?

我只能假設我讀過的那些文章來自未來,那時所有的開發者都是機器人。在那個世界裡,你的隊友會欣然接受對他們程式碼毫不修飾的批評,因為處理這類資訊會溫暖他們冰冷的機器人心。

我要大膽假設,你想在當下——隊友都是真人的現在——改善程式碼審查。我還要更大膽地假設,與同事保持正向的關係本身就是目的,而不只是用來將每個缺陷的成本降到最低而調整的變數。在這樣的情況下,你的審查方式會如何改變?

在本文中,我將討論一些技巧,將程式碼審查不僅視為技術流程,也視為一種社交互動。

什麼是程式碼審查?

「程式碼審查」一詞可以指涉一系列活動,從只是在隊友肩膀後面一起看程式碼,到二十人的會議逐行剖析程式碼。我在此使用這個詞來指稱一種正式且以書面進行,但又不像一系列面對面的程式碼檢視會議那樣繁重的流程。

程式碼審查流程

程式碼審查的參與者包括作者,也就是撰寫程式碼並送出審查的人,以及審查者,也就是閱讀程式碼並決定何時可以合併到團隊程式碼庫的人。一場審查可以有多位審查者,但為了簡化起見,我假設你就是唯一的審查者。

在程式碼審查開始之前,作者必須建立一個changelist(變更清單)。這是一組作者想要合併到團隊程式碼庫的原始碼變更。

審查在作者將 changelist 傳送給審查者時開始。程式碼審查以輪次進行。每一輪是作者與審查者之間一次完整的往返:作者送出變更,審查者則以書面回饋回應這些變更。每一場程式碼審查都包含一輪或多輪。

審查在審查者核准變更時結束。這通常稱為給予 LGTM,也就是「looks good to me(在我看來沒問題)」的縮寫。

為什麼這很困難?

如果一位程式設計師傳給你一份他自認很棒的 changelist,而你卻回覆了一長串理由說明它哪裡不好,這就是一個需要謹慎傳達的敏感訊息。

這就是我一點也不懷念資訊科技產業的原因之一,因為程式設計師是非常不討喜的一群人⋯⋯以航空業為例,那些嚴重高估自己技術水準的人都已經死了。

——Philip Greenspun(菲利普·葛林斯潘),ArsDigita 共同創辦人,摘自 Founders at Work(《創業現場》)

作者很容易將對其程式碼的批評,解讀為暗示自己是個不稱職的程式設計師。程式碼審查是分享知識、做出明智工程決策的機會。但如果作者將討論視為人身攻擊,這一切就無法實現。

彷彿這還不夠困難,你還得面對以文字傳達想法的挑戰,而文字溝通的誤解風險更高。作者聽不到你的語氣、看不到你的肢體語言,因此更需要謹慎地措辭。對於一位正感到防衛的作者來說,一句無害的註解像是「You forgot to close the file handle(你忘了關閉檔案控制代碼)」,也可能被解讀為「我真不敢相信你居然忘了關閉檔案控制代碼!你真是個笨蛋。」

技巧

  1. 讓電腦處理乏味的部分
  2. 用風格指南解決風格爭論
  3. 立即開始審查
  4. 由高層次著手,再逐步深入細節
  5. 不吝提供程式碼範例
  6. 絕不說「你」
  7. 將回饋表述為請求,而非命令
  8. 將意見繫於原則,而非個人好惡

讓電腦處理乏味的部分

在會議和電子郵件等干擾之間,你能專注於程式碼的時間本就稀少。你的腦力更是供不應求。閱讀隊友的程式碼在認知上相當耗費心力,需要高度專注。別把這些資源浪費在電腦就能做、而且做得更好的工作上。

空白字元錯誤就是一個明顯的例子。比較一下,由人工審查者找出縮排錯誤並與作者協作修正,與直接使用自動化排版工具相比,各自需要花費多少心力:

人工審查所需的心力使用排版工具所需的心力
  1. 審查者搜尋空白字元問題並發現縮排錯誤。
  2. 審查者撰寫註解指出縮排錯誤。
  3. 審查者重讀自己的註解,確保措辭清晰且不帶指責意味。
  4. 作者閱讀該註解。
  5. 作者修正程式碼的縮排。
  6. 審查者確認作者已妥善處理其註解。
完全不用!

右側之所以是空的,是因為作者使用的程式碼編輯器會在每次按下「Save」時自動排版空白字元。最糟的情況下,作者送出程式碼進行審查,而 continuous integration(持續整合) 系統會回報空白字元不正確。作者自行修正問題,審查者完全不需要操心。

留意你在程式碼審查中可以自動化的機械性工作。以下是常見的項目:

工作項目自動化解決方案
確認程式碼可建置continuous integration 解決方案,例如 TravisCircleCI
確認自動化測試通過continuous integration 解決方案,例如 TravisCircleCI
確認程式碼空白字元符合團隊風格程式碼排版工具,例如 ClangFormat(C/C++ 排版工具)或 gofmt(Go 排版工具)。
找出未使用的 imports 或未使用的變數程式碼檢查工具,例如 pyflakes(Python 檢查工具)或 JSLint(JavaScript 檢查工具)。

自動化能幫助你在擔任審查者時做出更有意義的貢獻。當你可以忽略某一整類問題,例如 imports 的排序或原始檔名稱的命名慣例,就能專注於更有趣的事物,例如功能性錯誤或可讀性方面的弱點。

自動化對作者也有好處。它能讓他們在幾秒鐘內而非幾小時後就發現粗心的錯誤。即時的回饋讓學習更容易、修正成本更低,因為作者腦中仍保有相關的脈絡。此外,如果他們必須聽到自己犯了愚蠢的錯誤,從電腦那裡聽到,遠比從你口中聽到更不會傷自尊。

與你的團隊合作,將這些自動化檢查直接整合到程式碼審查工作流程中(例如 Git 中的 pre-commit hooks(提交前掛鉤) 或 GitHub 中的 webhooks)。如果審查流程要求作者手動執行這些檢查,你就會失去大部分的好處。作者難免偶爾會忘記,這會迫使你繼續為那些本應由自動化處理的簡單問題進行審查。

用風格指南解決風格爭論

在審查中爭論風格是浪費時間。一致的風格固然重要,但程式碼審查不是爭辯大括號該放在哪裡的場合。要從審查中剔除風格爭論,最好的方法就是維護一份風格指南。

典型的風格爭論

一份好的風格指南不僅定義了命名慣例或空白字元規則等表層要素,也定義了如何使用特定程式語言的功能。舉例來說,JavaScript 和 Perl 充滿了各種功能——它們提供了許多種實作相同邏輯的方式。風格指南定義了做事的唯一正解,讓你不會落得一半團隊使用一種語言功能組合,而另一半卻使用截然不同組合的局面。

一旦有了風格指南,你就不需要浪費審查的往返去跟作者爭論誰的命名慣例比較好。直接依循風格指南,然後繼續前進即可。如果你的風格指南沒有針對某個特定問題訂定慣例,通常就不值得為此爭論。如果你遇到指南未涵蓋、但又重要到值得討論的風格問題,就跟整個團隊一起討論出結果。然後,將決定記錄到風格指南中,這樣你就再也不必為了同一件事爭論了。

選項 1:採用現成的風格指南

如果你在網路上搜尋,可以找到許多已公開、可直接套用的風格指南。Google’s style guides(Google 風格指南)是最廣為人知的,但如果這種風格不適合你,也可以找到其他的。採用現成的指南,你就能享有風格指南的好處,而無需承擔從零開始建立的龐大成本。

缺點是,各個組織會針對自身特定需求來最佳化風格指南。舉例來說,Google’s style guides 對於使用新語言功能的態度較為保守,因為他們擁有龐大的程式碼庫,程式碼必須能在從家用路由器到最新款 iPhone 的各種裝置上執行。如果你是一家只有四個人、單一產品的新創公司,你可能會選擇更積極地採用前沿的語言功能或擴充功能。

選項 2:逐步建立自己的風格指南

如果你不想採用現成的指南,也可以自行建立。每當在程式碼審查中出現風格爭論,就向整個團隊提出問題,以決定正式的慣例應該是什麼。達成共識後,就將該決定明文化到你的風格指南中。

我偏好將團隊的風格指南以 Markdown 形式放在版本控制之下(例如 GitHub pages)。如此一來,任何對風格指南的變更都會經過正常的審查流程——必須有人明確核准變更,團隊中的每個人也都有機會提出疑慮。Wiki 和 Google Docs 也是可接受的選項。

選項 3:混合式作法

結合選項 1 和選項 2,你可以採用一份現成的風格指南作為基礎,再維護一份在地化的風格指南來擴充或覆寫基礎指南。一個很好的例子是Chromium C++ style guide(Chromium C++ 風格指南)。它以Google’s C++ style guide(Google C++ 風格指南)為基礎,但在其上做了自己的修改與增補。

立即開始審查

將程式碼審查視為高度優先的事項。當你實際閱讀程式碼並提供回饋時,可以慢慢來,但要立即開始審查——理想上在幾分鐘內就開始。

程式碼審查接力賽

如果隊友傳給你一份 changelist,很可能代表在你的審查完成之前,他其他的工作都被卡住了。理論上,版本控制系統允許作者建立分支、繼續工作,然後再將審查中的變更正向合併到新的分支中。實際上,大概只有四位開發者能有效率地做到這件事。對其他人而言,要釐清 three-way diffs(三方差異比對) 得花上很久,久到抵銷了在等待審查回覆期間所取得的任何進展。

當你立即開始審查,就會創造出一個正向循環。你的審查週轉時間純粹取決於作者 changelist 的大小與複雜度。這會激勵作者送出小而範圍明確的 changelist。這些 changelist 對你而言更容易、也更愉快地審查,因此你審查得更快,循環便得以延續。

想像你的隊友實作了一項需要 1,000 行程式碼變更的新功能。如果他們知道你能在約 2 小時內審查完一份 200 行的 changelist,他們就可以將功能拆成每份約 200 行的 changelist,並在一兩天內讓整個功能完成提交。然而,如果無論大小,你都需要花上一天來完成所有程式碼審查,那麼現在要讓該功能完成提交就得花上一週。你的隊友不想空等一週,因此會被誘使送出更大的程式碼審查,例如每份 500 至 600 行。這些審查的成本更高,回饋品質也更差,因為要掌握 600 行變更的脈絡,遠比掌握 200 行變更來得困難。

一輪審查的絕對最長週轉時間應為一個工作天。如果你在處理更高優先順序的問題,無法在一天內完成一輪審查,請告知你的隊友,並給他們機會將其重新指派給其他人。如果你被迫每月超過一次以上推辭審查,很可能表示你的團隊需要放慢步調,才能維持健全的開發實務。

由高層次著手,再逐步深入細節

在某一輪審查中,你寫的註解越多,就越有可能讓作者感到不堪負荷。確切的上限因開發者而異,但危險區域通常從單輪審查中 20 至 50 則註解開始。

如果你擔心用大量的註解淹沒作者,請在前幾輪將自己限制在高層次的回饋。專注於像是重新設計類別介面或拆分複雜函式等問題。等到這些問題解決後,再處理較低層次的問題,例如變數命名或程式碼註解的清晰度。

一旦作者整合了你的高層次意見,你那些低層次的註解可能就會變得無關緊要。將它們延後到下一輪,你就能省去撰寫措辭謹慎註解來指出問題的繁重工作,也讓作者免於處理不必要的註解。這項技巧也能將你在審查期間關注的抽象層次分段,幫助你和作者以清晰、有系統的方式完成 changelist 的審查。

不吝提供程式碼範例

在理想的世界裡,程式碼作者會對收到的每一份審查心懷感激。這是他們學習的機會,也能保護他們免於犯錯。但在現實中,有許多外部因素可能導致作者對審查產生負面觀感,並因你給予意見而心生怨懟。也許他們正面臨期限壓力,因此除了你立刻蓋章核准之外的任何回應,都感覺像是阻撓。也許你們共事不多,因此他們不相信你的回饋是出於善意。

收到程式碼禮物

如果你透過寫出部分建議的變更來減輕作者的負擔,你就能展現身為審查者,你願意慷慨地投入時間。

舉例來說,想像你有一位不熟悉 Python 的 list comprehensions(串列生成式) 功能的同事。他們傳給你一份程式碼審查,其中包含以下幾行:

urls = []
for path in paths:
  url = 'https://'
  url += domain
  url += path
  urls.append(url)

如果你回應「Can we simplify this with a list comprehension?(我們可以用 list comprehension 來簡化這段嗎?)」,會惹惱他們,因為他們現在得花 20 分鐘去研究自己從未使用過的東西。

他們會更樂意收到像下面這樣的註解:

考慮像這樣用 list comprehension 來簡化:

urls = ['https://' + domain + path for path in paths]

這項技巧不限於一兩行的範例。我經常會建立自己的程式碼分支,向作者展示大型的概念驗證,例如拆分龐大的函式或新增單元測試以涵蓋額外的邊界案例。

將這項技巧保留給明確且無爭議的改進。在上述的 list comprehension 範例中,很少有開發者會反對減少 83% 的程式碼行數。相對地,如果你寫了一長串範例來展示基於個人好惡而認為「更好」的變更(例如風格變更),程式碼範例反而會讓你顯得強勢而非慷慨。

每輪審查將程式碼範例限制在兩到三個。如果你開始替作者重寫整份 changelist,就等於在暗示你認為他們沒有能力自己寫程式碼。

絕不說「你」

這一條聽起來可能有點奇怪,但請聽我說完:在程式碼審查中,絕不要使用「你」這個字。

你在審查中達成的決定,應該基於什麼能讓程式碼變得更好,而非是誰想出這個點子。你的隊友在他們的 changelist 上投入了大量心血,很可能對自己的成果感到自豪。他們聽到對其成果的批評時,自然的反應就是感到防衛與想要保護自己的作品。

以能將引發隊友防衛心的風險降到最低的方式來措辭你的回饋。明確表示你批評的是程式碼,而非寫程式碼的人。當作者在註解中看到「你」時,注意力就會從程式碼轉回到自身。這會增加他們將你的批評視為針對個人的風險。

試想這則看似無害的註解:

你把 ‘successfully’ 拼錯了。

作者可能會以兩種截然不同的方式解讀這則註解:

  • 解讀 1:嘿,好夥伴!你把 ‘successfully’ 拼錯了。不過我還是覺得你很聰明!這大概只是個打字錯誤吧。
  • 解讀 2:你居然把 ‘successfully’ 拼錯了,笨蛋。

對比一下省略「你」的註解:

sucessfully -> successfully

後者的註解只是單純的更正,而非對作者的評價。

幸好,要將你的回饋改寫成避免使用「你」並不困難。

選項 1:將「你」替換為「我們」

可以請將這個變數重新命名為更具描述性的名稱,例如 seconds_remaining 嗎?

改成:

我們可以將這個變數重新命名為更具描述性的名稱,例如 seconds_remaining 嗎?

「我們」強調了團隊對程式碼的共同責任。作者可能會離職到另一家公司,你也可能如此,但擁有這份程式碼的團隊仍會以某種形式繼續存在。當某件事顯然是期望作者自己去做時,說「我們」聽起來可能有點傻,但傻總比帶有指責意味來得好。

搬沙發漫畫

選項 2:從句子中移除主詞

另一種避免使用「你」的方法,是使用省略主詞的簡寫:

建議重新命名為更具描述性的名稱,例如 seconds_remaining

你也可以透過被動語態達到類似的效果。我在技術寫作中通常極力避免被動語態,但它可以是避開「你」的一種有用寫法:

這個變數應該被重新命名為更具描述性的名稱,例如 seconds_remaining

另一個選項是以問句的形式來表達,以「what about⋯⋯」或「how about⋯⋯」開頭:

要不要考慮將這個變數重新命名為更具描述性的名稱,例如 seconds_remaining

將回饋表述為請求,而非命令

程式碼審查比一般溝通需要更多的技巧與謹慎,因為討論很容易偏離主題,演變成私人爭執。你會期待審查者在審查中更加禮貌,但奇怪的是,我發現他們反而走向相反的方向。大多數人從不會對同事說:「把那個訂書機拿給我,然後去幫我買罐汽水來。」但我卻看過無數審查者以同樣強勢的命令來表述回饋,例如:「Move this class to a separate file.(把這個類別移到另一個檔案。)」

在回饋時,寧可過於溫和到令人覺得有點煩,也不要失禮。將你的註解表述為請求或建議,而非命令。

比較同一則註解以兩種不同方式表述的差異:

以命令形式表述的回饋以請求形式表述的回饋
Foo 類別移到另一個檔案。我們可以將 Foo 類別移到另一個檔案嗎?

人們喜歡對自己的工作擁有掌控感。對作者提出請求,能給予他們一種自主感。

請求也讓作者更容易禮貌地提出異議。也許他們的選擇有充分的理由。如果你將回饋表述為命令,作者的任何反駁聽起來都會像是抗命。如果你將回饋表述為請求或問題,作者就可以單純地回答你。

比較審查者如何措辭其最初的註解,會讓對話顯得對立或合作:

以命令形式表述的回饋(對立)以請求形式表述的回饋(合作)
審查者:將 Foo 類別移到另一個檔案。
作者:我不想那樣做,因為那樣它就會離 Bar 類別很遠。客戶幾乎總是會一起使用這兩者。
審查者:我們可以將 Foo 類別移到另一個檔案嗎?
作者:可以,但那樣它就會離 Bar 類別很遠,而且客戶通常會一起使用這兩個類別。你覺得呢?

看看當你編造想像的對話來證明自己的觀點將註解表述為請求而非命令時,對話會變得多麼文明?

將意見繫於原則,而非個人好惡

當你給作者一則註解時,請同時說明你建議的變更以及變更的原因。與其說「We should split this class into two.(我們應該把這個類別拆成兩個。)」,不如說「Right now, this class is responsible for both downloading the file and parsing it. We should split it up into a downloader class and parsing class per the single responsibility principle(單一職責原則).(目前這個類別同時負責下載檔案與解析檔案。根據單一職責原則,我們應該將其拆分為一個下載器類別與一個解析類別。)」

將你的註解立基於原則,能以具建設性的方式來引導討論。當你引用具體的原因,例如「We should make this function private to minimize the class’ public interface.(我們應該將這個函式設為私有,以最小化類別的公開介面。)」,作者就無法簡單地回應「No, I prefer it my way.(不,我比較喜歡我原本的方式。)」或者說,他們可以這樣回應,但那會顯得很可笑,因為你已經說明了這項變更如何達成某個目標,而他們只是表達了個人偏好。

軟體開發既是藝術也是科學。你並不總能用既定的原則來準確說明一段程式碼究竟哪裡出了問題。有時候程式碼就是醜陋或不直觀,很難說清楚原因。在這些情況下,盡你所能地解釋,但要保持客觀。如果你說「覺得這段很難理解。」那至少是一個客觀的陳述,相較於「這段很令人困惑。」,後者是一種價值判斷,對每個人來說未必都成立。

盡可能以連結的形式提供佐證。最理想的連結是你團隊風格指南中的相關章節。你也可以連結到程式語言或函式庫的文件。獲得高度肯定的 StackOverflow 答案也可以派上用場,但你離權威文件越遠,證據的可信度就越薄弱。

第二部分

如果你喜歡這篇文章,請參閱本文的下半部,該部分著重於如何在沒有難看衝突的情況下,讓審查順利收尾。其中包含以下技巧:

  • 處理過於龐大的程式碼審查,
  • 辨識給予讚美的機會,
  • 尊重審查的範圍,以及
  • 化解僵局。

如何像真人一樣進行程式碼審查(下篇)


Samantha Mason(薩曼莎·梅森) 編輯。插圖由 Loraine Yow(蘿蘭·尤)繪製。感謝 @global4g 對本文早期草稿提供寶貴的回饋。

原文由 Michael Lynch 發布

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