開原始碼評審的十個通用步驟 | Linux 中國

碼農談IT發表於2023-01-31

 
開原始碼評審的十個通用步驟 | Linux 中國
導讀:只要你遵循這些通用流程,程式碼評審並不可怕。                                       
本文字數:3400,閱讀時長大約:6分鐘

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

你是否需要在你還沒有完全理解整個專案時就對程式碼進行評審?抑或你避開了評審,以免讓你看起來不知道如何進行。

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

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

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

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

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


程式碼評審的目的是什麼?

你是否曾寫過一個非常簡單的補丁?你認為它是如此微不足道,不需要審查。或許你直接就合併了它。直到晚些時候,你意識到你犯了個錯誤,一個明顯的或是愚蠢的錯誤,比如錯誤的縮排,比如幾行重複的程式碼而不是呼叫函式(是的,這些都是經驗之談!)。

如果有其他人來審查程式碼,就會發現這些東西。

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

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


程式碼評審的通用流程

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


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

為什麼需要改動的解釋以及任何相關背景都應該被放在  資訊裡。如果沒有,請要求提供,並請投 -1 直到相關資訊被提供。

改動想解決的問題需要被解決嗎?它是專案應當關注的問題,還是與專案完全無關?


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

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


3、執行有改動和沒有改動的程式碼

我通常會在程式碼中設定幾個斷點,執行程式碼並檢查新程式碼是如何與其餘部分互動的。

如果你無法執行整個程式碼,試著將帶有新程式碼的函式複製到一個新的本地檔案,模擬輸入資料,然後執行。這在你不知道怎麼執行整個專案,或者無法接觸到執行所需的特殊環境時很有幫助。


4、新程式碼會破壞任何東西嗎?

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

以一個新的命令列選項為例,它會總是被目標所接受嗎?

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

或許新程式碼是匯入了新的東西。那麼這個新的庫,以及可能的新的依賴關係,能夠在老版本或者專案的執行系統中被找到嗎?

安全方面呢?新的依賴足夠安全嗎?你至少可以在網上快速地搜尋一下。還有,注意一下控制檯日誌裡的警告。有的時候在同一個庫裡也可以找到更安全的函式。


5、新程式碼是否有效?

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

檢查新程式碼的風格。它與專案的程式碼風格相匹配嗎?任何開源專案都(應該)有一份文件告知(新)貢獻者專案所遵循的風格和優秀實踐。

比如說,OpenStack 社群的所有專案都有一份 HACKING.rst 檔案。你經常也能找到一份包含所有必須知道的資訊。


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

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

(LCTT 譯註: 指程式設計中用來發現程式碼潛在錯誤和約束程式碼風格的工具,起源於 C 語言程式設計中的靜態分析工具 lint。“lint” 本意為衣服上積累的絨毛與灰塵,“lint” 的取名寓意則在於捕捉程式設計時產生的“絨毛與灰塵”)

(LCTT 校注:我建議,“Lint” 工具可以翻譯為 “程式碼清理” 或 “程式碼清潔” 工具。)

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


7、新的函式和方法是否必要?

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


8、有單元測試嗎?

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


9. 驗證重構

如果這次提交對現有程式碼進行了重構(它可能重新命名了某個變數,或者是改變了的變數的作用域,或者是透過加減引數來改變函式的足跡,又或者是刪去了某個東西),問一問你自己:

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

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

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

除了對專案倉庫的檢查外,檢查其他程式碼使用者也十分必要。如果有別的專案匯入了這個專案,它們可能也需要進行重構。在 OpenStack 社群中,我們有對應的工具來查詢別的社群專案。


10、專案文件是否需要做出更改?

你可以再一次使用  來檢查在專案文件中是否提到了相關的程式碼改動。用常識來判斷這次改動是否需要被收入文件以告知終端使用者,還是隻是一個不會影響使用者體驗的內部變化。


額外提示:考慮周到

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


最後幾句

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

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


via: 

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

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

LCTT 譯者 :yzuowei

來自 “ ITPUB部落格 ” ,連結:http://blog.itpub.net/70024924/viewspace-2933406/,如需轉載,請註明出處,否則將追究法律責任。

相關文章