6 個實用的 Code Review 實踐技巧

作者 | Alejandro Lujan

策劃 | 冬雨

編輯 | 萬佳

本文最初發布於 Shopify 博客，

經原作者授權由 InfoQ 中文站翻譯並分享。

Code reviews 是打造高效團隊的重要方面，這已經成為共識。關於這個主題，有許多文章曾經討論過，比如這篇論文——《 An Empirical Study of the Impact of Modern Code Review Practices on Software Quality 》。現實中，許多企業的無數團隊都進行過某種形式的 code reviews。

而實際情況是，code reviews 剛開始時，人們的激情高漲，之後，code reviews 則流於形式，或者要麼反饋不清晰、要麼讓人難以執行。長久以往，這讓團隊錯失了加快學習、分享知識的機會，最終難以提高代碼的質量。

在 Shopify，我們不僅立足長遠，而且希望追求發展更快。以我們的經驗來看，優秀的 code reviews 實踐對工程師的成長和我們所打造的產品質量有著巨大影響。

這樣一個場景相信很多人都很熟悉：

你剛剛加入一個新團隊，領導很快給你分配了一個編碼任務。作為新人，你特別想表現自己，因為你想秀一下自己的編碼水平。於是，你接下來做了這些事：

你為了完成任務瘋狂地敲了三周代碼；

你將一個包含大約 1000 行新代碼的 Pull Request 提交評審；

你收到兩條關於 code style 的評論，以及一個關於評審人表示他看不懂這些代碼用途的問題；

你修復 code style 並回答評審人的問題，然後評審人通過你寫的代碼；

你把代碼分支合併到 Master，雙眼緊閉，緊握著拳頭，緊咬牙關等待著結果。幾分鐘後，CI 完成。幸好，Master 沒有崩潰。然而…

此後 6 個月，你一直戰戰兢兢，不知道代碼何時會崩潰，以及以什麼方式崩潰。

你可能經歷過上述噩夢般的經歷，那我們談談怎樣改進這個流程吧！

在 Shopify，我們看重交付速度、學習以及長期發展。這些價值觀雖然有時會產生衝突，但卻引導我們不斷嘗試許多新技術，並推動團隊變革。

我在本文總結了一系列 Shopify 內部使用的實用技巧。藉助這些技巧，我們能交付經得起時間考驗的有價值的代碼。

術語說明：我們將 Pull Requests（PR）定義為合併到基礎分支前進行 code reviews 的一個工作單元。Github 和 Bitbucket 的用戶對這個術語很熟悉。

這個方法很簡單，可以成為提高 code reviews 工作流程最有用的技術。它之所以有效，主要有兩個原因：

將 PR 拆分為更小的代碼段，讓你有更多機會在更短時間內得到更深入的評審。

目前，我們無法設置一個適用於所有程式語言和所有類型工作的通用標準。對於內部的數據工程項目，我們原則上是要將 PR 控制在 200-300 行代碼。如果超過這個閾值，我們一般會將它拆分成更小的塊。

當然，我們也要注意不要將 PR 拆分得過小，因為這意味著評審人可能需要檢查好幾個 PR 才能理解整體邏輯。

你聽過造一輛汽車與畫一輛汽車的比喻嗎？這個比喻是這麼說的：

用戶要你造一輛車；

6 個月後，你造了一輛漂亮的保時捷；

你向用戶展示這輛車後，他們問你這輛車能不能放得下他們的 5 個孩子和衝浪板。

顯然，這裡的問題在於目標不清晰，團隊沒有收集到足夠的反饋就直接構建解決方案。如果在第一步後，我們先畫一幅汽車的草圖，並將其展示給用戶，他們會問相同的問題，這樣就可以進一步了解客戶需求。如此，就為我們節省了 6 個月的工作量。軟體也不例外，我們可能會犯同樣的錯誤，在用戶不需要的特性或模塊上投入大量工作。

在 Shopify，一般用 Work In Progress (WIP) PRs 來獲得早期反饋，其目標是驗證方向（算法、設計、API 等等選擇）。儘早變更可以避免在細節、修飾、文檔等方面浪費精力。

作為一名寫代碼的人，這意味著你要對變更工作方向持開放態度。

在 Shopify，我們信奉的原則是允許大家有自己的理解，但不固執己見。我們希望大家能在有充足理由的情況下自信地做出決定，但同時也能樂於學習其他更好的新方案。在實際工作中，我們使用 Github 的 Draft PRs，它們明確表明這項工作仍在流程中流轉，Github 不允許你合併一個 Draft PR。其他工具可能有類似的功能，至少你創建正常 PR 時可以加上一個 WIP 標籤，以明確表示該工作還處於前期階段。這將幫助你的評審人專注於適當的領域，提出適當的反饋。

除了行數外，需要考慮的另一個維度是你的工作單元試圖解決的問題數量。一個關注點可以是一個特性、一個錯誤修復、一個依賴項升級、一個 API 變更等等。你是否在重構的同時引入一個新特性？一次修復了兩個錯誤？同時引入了類庫升級和新的服務？

把 PR 分解為一個個單獨的關注點，它會產生下列影響:

更獨立的評審單元，這意味著更好的審查質量；

受影響的人更少，因此可以聚集在更少的幾個專業領域中；

原子性回滾，可以回滾小的 commit 或 PR。這是很有價值的，因為如果出了問題，就更容易確定錯誤是在哪裡引入的，以及回滾哪些部分。

將易事和難事分開。假設有一個新特性，需要重構一個頻繁使用的 API。你可以更改這個 API，升級十幾個調用的站點，然後實現這個特性。你的變更中有 80% 不是功能上的變更，明顯可以忽略掉，而 20% 是需要仔細注意測試覆蓋率、預期行為、錯誤處理等等的新代碼，並且可能要經過多次修訂。對於每一個修訂，評審人都需要瀏覽所有的修改以找到相關的部分。通過將其分成兩個 PR，很容易就可以快速完成大部分工作，並優化評審工作，將主要精力投入到難點上。

如果你最終拿到手的 PR 包含多個關注點，那麼你可以將其分解為多個單獨的塊。這樣能針對每一塊進行單獨的評審，每次評審的迭代周期可以更快，從而加速這個 PR 的總體評審周期。通常情況下，有一部分工作能先快速完成，避免代碼爛到不能用以及引起合併衝突。

將 PR 分解成單獨的關注點

上例的 PR 包含三個不同的關注點，我們將其進行拆分。可以看到，每個評審人需要檢查的上下文少了許多。最重要的是，只要其中任何一個部分的評審完成，代碼作者就能一邊等待其他評審反饋，一邊著手處理已經反饋的問題。在最極端的情況下，代碼作者會陸續收到各個部分的評審反饋，幾乎可以不間斷地處理完這一系列 PR，而不是完成初稿後，等上幾天（已經去忙其他的事），然後最後再返回頭來處理反饋意見。

專注於代碼，而不是人，這條實踐談的是人與人之間的溝通方式和關係。從根本上講，這是提倡我們嘗試把注意力集中在如何改進產品上，避免作者將評審意見當作對他個人的批評。

以下是一些你可以遵循的技巧：

評審人可以這樣想：「這是我們自己的代碼，我們該如何改進它呢？」

提出肯定意見！如果你看到有些代碼部分寫得不錯，就加條評論表揚一下。這能讓代碼作者繼續保持好的一面，並有助於他在心理上更容易接受改進建議。

代碼作者不妨這麼想，評審人的出發點肯定是好的，不要將評論當作是對個人的批評。

下表列出了一些存在不足的評審反饋，以及如何按以上建議進行重寫的建議。

歸根結底，code review 給我們提供了互教互學的機會，我們應該對此持開放歡迎的態度。

決定由誰來評審你的工作通常很有挑戰性。以下問題可作為參考：

無論你的團隊遵循哪些原則，請記住，作為一名代碼的作者，你有責任尋求並接受適當的人對你的代碼進行高質量的 code review。

最後但同樣非常重要的一點是，你的 PR 描述至關重要。這取決於你選擇的評審人，不同的人會有不同的上下文。代碼的作者有責任提供關鍵信息或更多上下文的連結，幫助評審人能夠反饋有價值的意見。

你可以把以下問題放到你的 PR 模板中：

為什麼這個 PR 是必要的？

誰會從中受益？

可能會出什麼問題？

你還考慮過其他方法嗎？你為什麼決定採用這種方法？

這對其他系統有什麼影響?

好的代碼不僅沒有錯誤，還非常有用。作為一名代碼的作者，請確保你的 PR 描述將代碼與團隊目標聯繫起來，最好能與待辦事項中的特性或缺陷描述聯繫起來。作為評審人，會先評審 PR 描述，如果它不夠完整，你是無法針對未定義的目標來判斷代碼是否適當的，不如在評審代碼前就把它打回去。請記住，有時代碼審查的最佳結果是認識到根本不需要這些代碼！

通過採用上面的一些技術，你可以在很大程度上影響軟體構建過程的速度和質量。但除此之外，還有潛在的文化影響：

任何團隊成員都應該能夠休上幾天假，他幾天不工作不會讓公司面臨風險，也不會因為擔心世界末日而不停地去看電子郵件。

如果你是團隊主管，不妨開始嘗試這些技巧，找出適合你所帶團隊的方法。

如果你是獨立貢獻者，可以與主管討論一下為什麼你認為代碼審查技術很重要，以及它如何提高效率和幫助團隊。

在下次一對一交流或團隊會議上，探討一下這個問題。

參考閱讀：

https://engineering.shopify.com/blogs/engineering/great-code-reviews

InfoQ Pro 是 InfoQ 專為技術早期開拓者和樂於鑽研的技術探險者打造的專業媒體服務平臺。

掃描下方二維碼關注 InfoQ Pro，獲取更多精彩內容。