歡迎光臨
每天分享高質量文章

如何做人性化的代碼審查?

(點擊上方公眾號,可快速關註)


編譯:精算狗,英文:Michael Lynch

最近,我一直在讀有關代碼審查最佳範例的文章。我註意到這些文章的關註點是找到 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)特征。他們給你發送了包含以下代碼的審查:

urls = []

for path in paths:

  url = ‘https://’

  url += domain

  url += path

  urls.append(url)

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

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

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

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

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

為清晰、無爭議的改進保留此技巧。在上面串列推導(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為這篇文章的早期版本提供寶貴的反饋。

覺得本文有幫助?請分享給更多人

關註「演算法愛好者」,修煉編程內功

赞(0)

分享創造快樂