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

本篇文章想要告訴你一個更好的方法。代碼評審code review

我還記得作為實習生加入 紅帽Red Hat

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

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

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

代碼評審的目的是什么？

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

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

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

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

代碼評審的通用流程

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

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

為什么需要改動的解釋以及任何相關背景都應該被放在 提交commit

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

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

額外提示：考慮周到

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

最后幾句

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