開源代碼評審的十個通用步驟

只要你遵循這些通用流程，代碼評審並不可怕。

你是否需要在你還沒有完全理解整個項目時就對代碼進行評審？抑或你避開了評審，以免讓你看起來不知道如何進行。

本篇文章想要告訴你一個更好的方法。 代碼評審 code review 並不需要你知道所有事情。實際上，就我個人經驗而言，這種情況非常普遍。

我還記得作為實習生加入 紅帽 Red Hat 的時候，被要求參與代碼評審。我們當時採取的是 +1 或 -1 的投票系統，而我在一開始的時候常常躊躇於該如何評審。我發現我會問自己，如果我對於一處改動給予了 +1，而別人卻投了 -1，我是不是看起來很蠢？

如果你對一處改動投了 +1，而別人投了 -1，這又意味著什麼呢？答案是不意味任何事！你可能只是漏掉了一處別人注意到的細節。這不意味著世界末日。這也是為什麼我們會用投票系統。正如同所有開源項目一樣，代碼合併是一項協同工作。

最近，我接到了太多的代碼評審工作，以至於我幾乎做不過來。我同時也注意到，參與評審的貢獻者數量正在穩步減少。

出於這個原因，我想要寫一篇文章闡述我對代碼評審的個人觀點。在這篇文章里，我會分享一些訣竅與技巧。我將會向你展示幾個用來問自己的問題，以及在評審代碼時需要注意的一些地方。

代碼評審的目的是什麼？

你是否曾寫過一個非常簡單的補丁？你認為它是如此微不足道，不需要審查。或許你直接就合併了它。直到晚些時候，你意識到你犯了個錯誤，一個明顯的或是愚蠢的錯誤，比如錯誤的縮進，比如幾行重複的代碼而不是調用函數（是的，這些都是經驗之談！）。

如果有其他人來審查代碼，就會發現這些東西。

代碼評審的一個目的便是為你帶來一雙新的眼睛，從新的視角看待你要嘗試解決的問題。這種新的背景也正是為什麼代碼評審至關重要。

你可能認為你必須是一個語言專家，才能審查別人的代碼、項目，或兩者。讓我來告訴你一個所有代碼評審者都想跟你說的秘密吧：大錯特錯！你並不需要完全理解該項目或者編程語言，就可以為一個改動提供全新的視角。下面，我將向你展示代碼評審的通用流程。

代碼評審的通用流程

這是我的代碼評審流程，拆分成了幾個要點。這個流程包含了我會問自己的一些問題，以幫助我專註於代碼的變化以及其後果。你不需要嚴格依照這個順序來進行評審。如果有任何原因導致你無法執行其中的某一步，跳過那一步就好。

1、理解改動，它想要解決的問題，以及為什麼要這麼做

為什麼需要改動的解釋以及任何相關背景都應該被放在 提交 commit 信息里。如果沒有，請要求提供，並請投 -1 直到相關信息被提供。

改動想解決的問題需要被解決嗎？它是項目應當關注的問題，還是與項目完全無關？

2、你會如何實現解決方案？它會不一樣嗎？

在這個時候，你應該已經知道代碼改動是為了什麼。換做是你會怎麼做？在進一步對改動進行細節評審前，先思考這個問題。如果你想出了一個不一樣的解決方案，並且你認為你的方案更好，在評審中提出來。你不需要投 -1；去問問作者為什麼沒有往那個方向走，看看這次討論會把你們帶向何方。

3、運行有改動和沒有改動的代碼

我通常會在代碼中設置幾個斷點，運行代碼並檢查新代碼是如何與其餘部分互動的。

如果你無法運行整個代碼，試著將帶有新代碼的函數複製到一個新的本地文件，模擬輸入數據，然後運行。這在你不知道怎麼運行整個項目，或者無法接觸到運行所需的特殊環境時很有幫助。

4、新代碼會破壞任何東西嗎？

我是說，任何東西。想一想可能的後果。

以一個新的命令行選項為例，它會總是被目標所接受嗎？

是否存在這樣一種情況，使得新選項無法被接受或是會與其他東西起衝突？

或許新代碼是導入了新的東西。那麼這個新的庫，以及可能的新的依賴關係，能夠在老版本或者項目的運行系統中被找到嗎？

安全方面呢？新的依賴足夠安全嗎？你至少可以在網上快速地搜索一下。還有，注意一下控制台日誌里的警告。有的時候在同一個庫里也可以找到更安全的函數。

5、新代碼是否有效？

你剛剛確認了被提出的解決方案大概是正確的。現在該檢查代碼本身了。你需要關注代碼的有效性和必要性。

檢查新代碼的風格。它與項目的代碼風格相匹配嗎？任何開源項目都（應該）有一份文檔告知（新）貢獻者項目所遵循的風格和優秀實踐。

比如說，OpenStack 社區的所有項目都有一份 HACKING.rst 文件。你經常也能找到一份新貢獻者指南包含所有必須知道的信息。

6、確認所有新增的變數和導入都被使用

你正在評審的代碼常常已經過多次迭代，有的時候代碼的最終版本與初始版已迥然不同。所以我們很容易忘記一些在歷史版本中加入的變數與引用。自動化檢測通常會用到 lint 工具，類似 Python 中的 [flake8][12]。

（LCTT 譯註：lint 指編程中用來發現代碼潛在錯誤和約束代碼風格的工具，起源於 C 語言編程中的靜態分析工具 lint。「lint」 本意為衣服上積累的絨毛與灰塵，「lint」 的取名寓意則在於捕捉編程時產生的「絨毛與灰塵」）

（LCTT 校註：我建議，「Lint」 工具可以翻譯為 「代碼清理」 或 「代碼清潔」 工具。）

你可以在不聲明新變數的情況下重寫代碼嗎？通常情況下你可以，但問題是這樣是否更好。這會帶來什麼益處嗎？我們的目標不是要創造儘可能多的單行代碼，而是寫出高效且易讀的代碼。

7、新的函數和方法是否必要？

項目里的別的地方是否存在可以被複用的功能類似的函數？確保避免重新發明輪子以及重新實現已經被定義的邏輯永遠都是值得的。

8、有單元測試嗎？

如果補丁增加了新的函數或者在函數內添加了新的邏輯，它也應該附帶對應的單元測試。新函數的作者總是比別人更適合寫該函數的單元測試。

9. 驗證重構

如果這次提交對現有代碼進行了重構（它可能重命名了某個變數，或者是改變了的變數的作用域，或者是通過加減參數來改變函數的足跡，又或者是刪去了某個東西），問一問你自己：

• 這個可以被刪除嗎？它會影響到穩定分支嗎？
• 所有出現的地方都刪掉了嗎？

你可以利用 grep 命令 來查找。你不會相信有多少次我投 -1 就是因為這個。這是一個任何人都會犯的簡單錯誤，也正因如此任何人都可以發現它。

提交的所有者很容易忽略這些事情，這完全可以理解。我也犯過很多次這種錯誤。我最終發現問題的根源在於我太急於提出評審，以至於我忘記了對倉庫進行整體檢查。

除了對項目倉庫的檢查外，檢查其他代碼用戶也十分必要。如果有別的項目導入了這個項目，它們可能也需要進行重構。在 OpenStack 社區中，我們有對應的工具來查詢別的社區項目。

10、項目文檔是否需要做出更改？

你可以再一次使用 grep 命令 來檢查在項目文檔中是否提到了相關的代碼改動。用常識來判斷這次改動是否需要被收入文檔以告知最終用戶，還是只是一個不會影響用戶體驗的內部變化。

額外提示：考慮周到

當你在評審完新代碼後提出建議或評論時，要考慮周到，反饋準確，描述詳盡。如果有你不理解的地方就發出提問。如果你認為代碼存在錯誤，解釋你的理由。記住，如果作者不知道什麼地方出了問題，他們就無法修復它。

最後幾句

唯一的壞評審是沒有評審。通過評審和投票，你提供了你的觀點並為此投票。沒有人指望你來做出最終決定（除非你是核心維護者），但是投票系統允許你提供你的觀點和意見。相信我，補丁所有者會很高興你這麼做了的。

你能想到別的要點來給出好的評審嗎？你是否有我不知道的特殊技巧？在評論中分享它們吧！

via: https://opensource.com/article/22/10/code-review

作者：Martin Kopec 選題：lkxed 譯者：yzuowei 校對：wxy

本文由 LCTT 原創編譯，Linux中國 榮譽推出

本文轉載來自 Linux 中國: https://github.com/Linux-CN/archive