如何做人性化的程式碼審查?

精算狗發表於2018-05-30

最近,我一直在讀有關程式碼審查最佳範例的文章。我注意到這些文章的關注點是找到 bug,而忽略了程式碼審查其他的部分。用建設性、專業的問題溝通方式?不相關!只要識別出所有的 bug,剩下的部分會水到渠成。

我只能假設我讀過的這些文章都來自未來,那時候所有的開發人員都是機器人。在那個世界,你的隊友歡迎對其程式碼未經過推敲措辭的批評,因為處理這樣的資訊能溫暖他們冰冷的機器人之心。

我要做一個大膽的假設,你想要在當前世界改進程式碼審查,此時你的隊友都是人類。我還要做一個更大膽的假設,你與同事之間積極的關係本身就是一個目的,而不僅僅是一個可調整的變數來最小化缺陷的平均成本。在這些情況下,你的審查實踐會發生怎樣的變化呢?

在這篇文章中,我討論了一些技巧,把程式碼審查既看作是技術過程,也看作是社會過程。

什麼是程式碼審查?

“程式碼審查(code review)”這一術語可以指一系列活動,從簡單地站在隊友身後讀讀程式碼,到 20 人與會的單行程式碼分析。我用這一術語指正式的、書面的過程,但也不像一系列現場程式碼審查會議那麼重大。

如何做人性化的程式碼審查?

程式碼審查的參與者包括作者以及審查者:作者寫程式碼並把程式碼送去審查,審查者讀程式碼並決定程式碼什麼時候就緒併入團隊的程式碼庫。一次審查可以由多個審查者完成,但是我做了簡化的假設——你是唯一的審查者。

在程式碼審查開始之前,作者必須建立一個變更表。作者想要將原始碼併入團隊程式碼庫,變更表包括一系列原始碼的變更。

當作者把變更表發給審查者時,審查就開始了。程式碼審查是迴圈發生的。每個迴圈都是作者與審查者之間完整的往返:作者傳送變更,審查者給予變更的書面反饋。每次程式碼審查都包括一次或者更多的迴圈。

當審查者批准了這些變更,審查結束。這通常指的是給出 LGTM,“我覺得不錯(looks good to me)”的簡寫。

這為什麼很難?

如果程式設計師給你發了一份變更表,他們覺得這個變更表棒極了。你又給他們寫了一份詳細的清單,解釋為什麼這個變更表並不好。這是需要小心處理的資訊。

這是我不想念 IT 的一個原因,因為程式設計師是非常不可愛的人……比如,在航空業,那些過分高估了自己技術水平的人都死了。

Philip Greenspun,ArsDigita 的聯合創始人,引自《Founders at Work》。

作者容易把對其程式碼的批評解讀為暗示他們不是合格的程式設計師。程式碼審查是一個分享知識和做工程決定的機會。但是如果作者把討論理解為個人攻擊,這個目標無法達成。

除此之外,你還面臨著書面傳達想法的挑戰,詞不達意的風險會更高。作者聽不到你的語氣,也看不到你的肢體語言,所以清晰地、小心地傳達你的反饋更為重要。對一個有戒備心的作者來說,一句無冒犯意味的批註,比如“你忘了關閉檔案控制程式碼”,可以被理解成“真不敢相信你忘了關閉檔案控制程式碼!你真是個傻子。”

技巧

  1. 讓電腦做無聊的部分
  2. 用風格指南平息風格爭論
  3. 馬上開始審查
  4. 從高階別開始,逐步向下
  5. 慷慨地使用程式碼示例
  6. 永遠別說“你
  7. 把反饋表達成請求,而不是指令
  8. 把批註與原則聯絡在一起,而不是觀點

讓電腦做無聊的部分

在會議和郵件的干擾下,可用來專注於程式碼的時間很少。你的精神毅力更是短缺。讀隊友的程式碼是認知上的負擔,要求高強度的專注。別把這些資源浪費在電腦能做的任務上,尤其是當電腦能做得更好的時候。

空白錯誤是一個顯著的例子。比較一下人類審查者找到縮排錯誤並與作者一起改正所花費的精力,和僅僅使用一個自動排版工具所花費的精力:

人類審查者需要的精力 排版工具需要的精力
1.審查者尋找空白錯誤,找到錯誤的縮排

2.審查者寫批註,指出錯誤縮排

3.審查者重新讀批註,確保措辭清晰,不含指責意味

4.作者讀批註

5.作者改正程式碼縮排

6.審查者核實作者適當地處理了批註

無!

右邊是空的,因為作者用了一個程式碼編輯器,每次他們點選“儲存”時,該程式碼編輯器會自動規定空白的格式。在最糟的情況下,作者把程式碼發出去以供審查,持續整合解決方法報告說空格錯誤。作者在不需要審查者顧慮的情況下,修正這個問題。

在程式碼審查中尋找可以被自動解決的機械性任務。以下是常見的例子:

任務 自動解決方法
驗證程式碼的構建 持續整合方法,比如 Travis 或者 CircleCI
證實通過了自動測試 持續整合方法,比如 Travis 或者 CircleCI
驗證程式碼空白與團隊風格一致 程式碼排版器,比如 ClangFormat (C/C++ 排版器) 或者 gofmt (Go 排版器)
識別未使用的輸入或者變數 程式碼 linter,比如 pyflakes (Python linter) 或者 JSLint (JavaScript linter)

自動化使你作為審查者能做出更多有意義的貢獻。當你能忽略一整個類別的問題,比如輸入的排序或者原始檔命名的約定,你能夠關注更有趣的事情,比如函式錯誤或者可讀性缺陷。

自動化也能給作者帶來好處。自動化使作者用幾秒鐘發現粗心的錯誤,而不是幾小時。即時反饋使得從錯誤中學習更容易,修正錯誤的代價也更小,因為作者腦海中還有相關的背景。另外,如果他們不得不聽到自己犯下的愚蠢錯誤,對自尊心來說,從電腦那聽到要比從你那聽到更容易被接受。

和你的團隊一起將這些自動檢查加入程式碼審查的工作流程中(例如,在 Git 中的 pre-commit hooks 或者 Github 中的 webhooks)。如果審查過程要求作者手動執行這些檢查,你會損失大部分好處。作者總是會忘記一些情況,迫使你繼續審查簡單的問題,而這些問題本來就能被自動處理。

用風格指南平息風格爭論

關於風格的爭論浪費了審查的時間。一致的風格確實重要,但是程式碼審查不是爭論花括號位置的時候。在審查中消除風格爭論的最佳辦法是,遵守一個風格指南。

如何做人性化的程式碼審查?

好的風格指南不僅定義了像命名習慣或者空白規則這樣的表面元素,而且定義了怎樣使用給定程式語言的特徵。比如,JavaScript 和 Perl 都包含了一些功能——他們提供了許多實現相同邏輯的方法。風格指南定義了做事的唯一方法,這樣不會以一半隊員用了一組語言特徵而另一半隊員用了完全不同的一組特徵收尾。

一旦有了一個風格指南,你就不需要浪費審查迴圈,來跟作者爭論到底誰的命名習慣最好。只要遵從風格指南然後繼續就行。如果你的風格指南沒有指定某個特定問題的約定,那它一般都不值得爭論。如果你遇到一個風格指南未涉及的問題,它又重要到需要討論,和團隊一起推敲。然後把決定加到風格指南,這樣你們永遠不需要再進行一次這個討論。

選擇 1:採納一個現存的風格指南

如果從網上搜尋,你能找到已釋出的風格指南可供使用。Google 的程式設計風格指南是最知名的,但是如果它的風格不適合你,你可以找到其他的指南。通過採納一個現存的指南,不需要從頭創造一個風格指南的大量花費就能繼承其好處。

壞處是組織為他們自己特別的需要優化其風格指南。比如,Google 的風格指南在使用新語言特徵上比較保守,因為他們有一個巨大的程式碼庫,其中的程式碼要在所有東西上執行,從家用路由器到最新的 iPhone。如果你們是一個只有一個產品的四人小組,你可能選擇在使用前沿語言特徵或者擴充套件時更大膽。

選擇 2:不斷創造你自己的風格指南

如果你不想採納現存的指南,你可以自己創造一個。在程式碼審查中每產生一次風格爭論,向整個團隊提問來決定官方約定應該是什麼。當你們達成共識,把決定編進風格指南中。

我傾向於將團隊的風格指南作為源控制下的 Markdown(例如 GitHub 頁面)。這樣,對風格指南的任何改動都需要通過普通的審查過程——某人得明確批准改動,而且團隊中的每個人都有提出疑慮的機會。Wikis 和 Google 檔案都是可接受的選擇。

選擇 3:混合方法

合併選擇 1 和選擇 2,你可以採納現存的風格指南作為基礎,然後用本地風格指南來擴充套件或者覆蓋這個基礎。一個好例子是 Chromium 的 C++ 風格指導。它用 Google 的 C++ 風格指導作為基礎,但是在其上添上自己的改動和附加。

馬上開始審查

將程式碼審查視為高優先順序。當你真正閱讀程式碼並反饋時,慢點來,但是要馬上開始審查——最好在幾分鐘內開始。

如何做人性化的程式碼審查?

如果隊員發給你一個變更表,這可能意味著直到你完成審查前,他們會卡在其他工作上。理論上,源控制系統使作者能建起新的分支,繼續工作,然後從審查中把變動合併進新分支。實際上,一共有大約四個開發者能夠高效地做這件事。其他人要花很長時間來清理三方差異,以致於抵消掉了等待審查完成這段時間裡的進步。

你馬上開始審查,就創造了一個良性迴圈。你的審查時間完全變成了一個與作者的變更表大小和複雜度相關的函式。這激勵作者傳送短小、範圍狹窄的變更表。對你來說這樣的變更表審查起來更容易,也更愉悅,所以你能更快地審查,迴圈繼續。

想象一下你的隊員要執行一個新特徵,這個特徵要求 1000 行程式碼變更。如果他們知道你能在大概 2 小時內完成一個 200 行的變更表的審查,他們可以把特徵拆分成各包含 200 行的變更表,然後在一兩天內檢查完整個特徵。但是,如果無論大小你都要花一天來完成所有的程式碼審查,現在就要花一週時間才能檢查完整個特徵。你的隊員不想傻坐一週,所以他們被激勵著去傳送更大的程式碼審查,比如每個包含 500 到 600 行。這樣審查起來花銷更大,反饋也更差,因為記 600 行變更表的背景要比 200 行變更表難。

一個審查迴圈的最大週期應該是一個工作日。如果你正在處理一個更高優先順序的問題,不能在一天內完成一個審查迴圈,讓你的隊員知悉並給予他們把審查交給別人的機會。如果你一個月被強制回絕審查超過一次,可能意味著你的團隊需要放慢腳步,這樣你能保持理智的開發實踐。

從高階別開始,逐步向下

在一個既定的審查迴圈中,你寫的批註越多,讓作者感覺受打壓的風險越大。準確的界限隨開發者的不同而不同,但是一個審查迴圈中 20 到 50 個批註一般是危險區的開始。

如果你擔心把作者淹沒在批註的海洋裡,約束你自己在早期迴圈中反饋高階別的問題。注意重新設計類介面或者拆分複雜函式這樣的問題。等到這些問題都解決了再去處理低階別的問題,比如變數命名或者程式碼評論的清晰度。

一旦作者整合了你高階別的批註,低階別的批註可能會變得無意義。把低階別的批註推遲到後期的迴圈中,你可以把自己從小心措辭的工作中解救出來,也免得作者處理不必要的批註。這個技巧也細分了審查過程中你所關注的抽象層,幫助你和作者用清晰、系統的方法完成變更表。

慷慨地使用程式碼示例

在一個理想的世界裡,程式碼作者會感謝收到的每一次審查。這是他們學習的一個機會,也能防止他們犯錯。事實上,有許多外部因素能導致作者負面地解讀審查,怨恨你給他們批註。可能他們正面臨著截止日期的壓力,所以除了立刻不經審查的批准以外的東西都感覺像阻礙。可能你們沒怎麼在一起工作過,所以他們不相信你的反饋是好意的。

一個讓作者對審查過程感覺良好的方法是,在審查中找機會送他們禮物。所有開發者都愛收到的禮物是什麼呢?當然是程式碼示例啦。

如何做人性化的程式碼審查?

如果通過寫一些建議的改動來減輕作者的負擔,就證明了作為審查者,你對時間很慷慨。

比如,想象一下你的一個同事不熟悉 Python 的列表推導(list comprehension)特徵。他們給你傳送了包含以下程式碼的審查:

回覆“能用列表推導(list comprehension)簡化這個嗎?”會使他們苦惱,因為現在他們得花 20 分鐘搜尋他們之前從沒用過的東西。

收到像以下這樣的批註他們會更開心:

考慮用像這樣的列表推導(list comprehension)來進行簡化:

這個技巧並不侷限於單命令程式。我會經常建立我自己的程式碼分支,向作者展示概念的一個大型證明,比如拆分一個大型函式或者增加一個單元測試來覆蓋一個附加邊界情況。

為清晰、無爭議的改進保留此技巧。在上面列表推導(list comprehension)示例中,極少有開發者會拒絕減少 83% 的程式碼行數。相反,如果你寫了一個冗長的示例來演示某個變動“更好”,而這個變動是基於你自己的個人品味(比如,風格變動),程式碼示例讓你看起來固執己見,而不是慷慨大方。

限制你自己在每個審查迴圈中只寫兩到三個程式碼示例。如果你開始為作者寫整個變更表,這標誌著你覺得作者沒能力寫自己的程式碼。

永遠別說“你”

這聽起來挺怪異的,但是聽我說:永遠別在程式碼審查中使用“你”這個字。

在審查中做的決定應該是基於什麼能讓程式碼更好,而不是誰出的主意。你的隊員在他們的變更表中傾注了大量心血,而且很可能為自己的工作感到驕傲。他們聽到對其工作的批評,自然反應是擺出防禦和保護的姿態。

組織反饋所使用的措辭,以最小化激起隊員戒備心的風險。講清楚你是在批評程式碼,而不是程式設計師。當作者在評論中看到“你”這個字,會將他們的注意力從程式碼轉移到自己身上。這增加了他們把批評私人化的風險。

考慮一下這個無害的評論:

你拼錯了“successfully”。

作者可以把這個批註理解成兩種不同的意思:

  • 理解 1:嗨,好傢伙!你拼錯了“successfully”。但是我還是覺得你聰明!那可能就是個筆誤。
  • 理解 2:你拼錯了“successfully”,笨蛋。

把這個跟省略了“你”的批註比較一下:

sucessfully -> successfully

後者是一個簡單的修正而不是對作者的審判。

幸運地是,在重新寫反饋時避免使用“你”並不難。

選擇 1:用“我們”替換“你”

能重新命名這個變數,讓它更具有描述性嗎?比如 seconds_remaining

變成:

我們能重新命名這個變數,讓它更具有描述性嗎?比如 seconds_remaining

“我們”加強了團隊對程式碼的集體責任。作者可能跳槽到一個不同的公司去,你也可能,但是擁有這個程式碼的團隊會一直以不同的形式存在。當你明顯期望作者自己做某些事的時候,說“我們”聽起來會比較傻,但是傻要比指責好。

如何做人性化的程式碼審查?

選擇 2:移除句子的主語

另一個避免使用“你”的方法是用省略句子主語的簡化句子:

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

你可以用被動語態實現相似的效果。我在技術寫作中一般會避免像瘟疫一樣使用被動語態,但是它是個有用的方法來避免使用“你”。

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

另一個選擇是把它表述為一個問題,用“……如何”或者“……怎麼樣”開頭:

把變數重新命名為更具有表述性的名稱怎麼樣?比如 seconds_remaining

把反饋表達成請求,而不是指令

程式碼審查相對平常的交流來說,要求更多的機智和謹慎,因為存在高風險把討論轉變成私人爭論。你會期望審查者在審查中表示出禮貌,但是奇怪地是,我發現他們走向了另一個方向。多數人永遠不會對同事說“給我訂書機,再給我拿瓶汽水。”但是我看到過無數審查者用類似的指令來表達反饋,比如,“把這個類移到一個單獨的檔案裡。”

寧可在反饋中惱人地紳士。把批註表達成請求或者建議那樣,而不是指令。

比較用兩種不同方式表達的同一個批註:

表達成指令的反饋 表達成請求的反饋
把 Foo 類移到一個單獨的檔案裡。 我們能把 Foo 類移到一個單獨的檔案裡嗎?

人們喜歡掌控自己的工作。向作者提出請求給他們帶來自主意識。

請求也讓作者禮貌地反饋更容易。可能他們的選擇是有合理的。如果把反饋表達成指令,來自作者的任何反饋都像違反指令。如果你把反饋表達成請求或者問題,作者能簡單地回答你。

比較對話的好鬥程度,取決於審查者怎麼表達他們的初始批註:

表達成指令的反饋(好鬥的) 表達成請求的反饋(合作的)
審查者:把 Foo 類移到單獨的檔案裡
作者:我不想這麼做,因為那樣就離 Bar 類太遠了。客戶幾乎總會一起呼叫他們。
審查者:我們能把 Foo 類移到單獨的檔案裡嗎?
作者:可以,但是那樣就離 Bar 類太遠了,以及客戶一般會一起使用這兩個類。你覺得呢?

看看當你構建虛擬對話來證明觀點把批註表達成請求而非指令的時候,對話變得多麼有禮貌。

把批註與原則聯絡在一起,而不是觀點

當你給作者寫批註時,既要給出變更建議,也要給出變更的理由。“現在,這個類既負責下載檔案,也負責解析檔案。我們應該依照單一責任原則,把它拆分成一個下載類和一個解析類。”這麼說會更好,而不是說“我們應該把這個類分成兩個。”

讓你的批註有原則性的立足點,這樣能讓討論走向更積極的方向更有建設性。但你有一個具體的原因,比如“我們應該把這個函式寫成私有函式,來最小化 public 藉口類”,作者就不能簡單地回覆“不,我傾向於我的方法。”更確切地說,他們可以,但是因為你演示了改動如何滿足目標,而他們只陳述了一個偏好,他們會看起來很傻。

軟體開發既是藝術也是科學。你不可能永遠都能用確定的原則來明確表達程式碼到底哪裡出了問題。有時候程式碼只是難看或者不符合直覺,不容易確定為什麼。在這些情況下,解釋你能怎麼做,但是保持客觀性。如果你說“發現這不容易理解”,這至少是個客觀的陳述;相反,“莫名其妙”是一個價值判斷,不一定適用於所有人。

儘可能以連結的形式提供支援證據。團隊風格指南的相關部分是你能提供的最佳連結。你也可以連結到語言或者庫的檔案。高票 StackOverflow 回答也行,但是離權威檔案越遠,你的證據變得越不穩固。

第二部分:即將上線

敬請期待其他小技巧,包括:

  • 處理特別大的程式碼審查
  • 識別給予表揚的機會
  • 尊重稽核的,以及
  • 化解僵局

Samantha Mason 編輯。插圖來自 Loraine Yow。感謝 @global4g為這篇文章的早期版本提供寶貴的反饋。

相關文章