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

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

我還記得作為實習(xí)生加入 紅帽Red Hat

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

最近，我接到了太多的代碼評審工作，以至于我?guī)缀踝霾贿^來。我同時也注意到，參與評審的貢獻者數(shù)量正在穩(wěn)步減少。

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

代碼評審的目的是什么？

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

如果有其他人來審查代碼，就會發(fā)現(xiàn)這些東西。

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

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

代碼評審的通用流程

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

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

為什么需要改動的解釋以及任何相關(guān)背景都應(yīng)該被放在 提交commit

改動想解決的問題需要被解決嗎？它是項目應(yīng)當(dāng)關(guān)注的問題，還是與項目完全無關(guān)？

2、你會如何實現(xiàn)解決方案？它會不一樣嗎？

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

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

我通常會在代碼中設(shè)置幾個斷點，運行代碼并檢查新代碼是如何與其余部分互動的。

如果你無法運行整個代碼，試著將帶有新代碼的函數(shù)復(fù)制到一個新的本地文件，模擬輸入數(shù)據(jù)，然后運行。這在你不知道怎么運行整個項目，或者無法接觸到運行所需的特殊環(huán)境時很有幫助。

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

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

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

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

或許新代碼是導(dǎo)入了新的東西。那么這個新的庫，以及可能的新的依賴關(guān)系，能夠在老版本或者項目的運行系統(tǒng)中被找到嗎？

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

5、新代碼是否有效？

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

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

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

6、確認(rèn)所有新增的變量和導(dǎo)入都被使用

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

（LCTT 譯注：??lint?? 指編程中用來發(fā)現(xiàn)代碼潛在錯誤和約束代碼風(fēng)格的工具，起源于 C 語言編程中的靜態(tài)分析工具 ??lint??。“l(fā)int” 本意為衣服上積累的絨毛與灰塵，“l(fā)int” 的取名寓意則在于捕捉編程時產(chǎn)生的“絨毛與灰塵”）

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

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

7、新的函數(shù)和方法是否必要？

項目里的別的地方是否存在可以被復(fù)用的功能類似的函數(shù)？確保避免重新發(fā)明輪子以及重新實現(xiàn)已經(jīng)被定義的邏輯永遠都是值得的。

8、有單元測試嗎？

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

9. 驗證重構(gòu)

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

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

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

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

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

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

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

額外提示：考慮周到

當(dāng)你在評審?fù)晷麓a后提出建議或評論時，要考慮周到，反饋準(zhǔn)確，描述詳盡。如果有你不理解的地方就發(fā)出提問。如果你認(rèn)為代碼存在錯誤，解釋你的理由。記住，如果作者不知道什么地方出了問題，他們就無法修復(fù)它。

最后幾句

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