同行代碼評審過程中的實踐經驗
原文出處: salsitasoft blog 譯文出處:博客在線
數百萬年前,猿從樹上下來,進化出了對生拇指,最終,變成了人類。
我們以類似的眼光來看下強制性代碼評審(Code Review):好像是一種能在軟件開發這塊廣闊的領域里將人類從獸里分離出來的東西。
不過,我有時候會從我們的團隊成員里聽到下面這樣的評論:
“這個項目的代碼評審根本就是浪費時間。”
“我沒有時間做代碼評審。”
“我的項目發布延期了,都是因為我那懦弱的同事還沒有做任何評審。”
“你能相信我的同事竟想讓我在代碼中改點東西嗎?請向他們解釋:如果我那最初的優雅代碼受到任何方式改動的話,那就意味著宇宙微妙的平衡將要遭到破壞。”
為什么我們要做代碼評審?
首先,讓我們謹記為什么要做代碼評審。對于任何專業的軟件開發人員來說,最重要的目標之一是能夠持續的提高他們的工作質量。即使你的團隊里盡是優秀的程序員,你也不能將你自己與一個有能力的自由從業者區分開來,除非你能夠作為一個團隊工作。代碼評審是達到這個目的的最重要方式之一。尤其,它們:
給予你第二雙眼睛來找到做某些事的瑕疵和更好的方法。
確保至少有一個其他人員熟悉你的代碼。
通過向新員工展示更有經驗的開發者的代碼來幫助訓練他們。
通過讓評審者和被評審者互相展示好的想法和做法以促進知識分享。
鼓勵開發者在他們的工作中更加盡心盡力,因為他們知道自己的代碼將來要被他們的的某個同事評審。
做徹底深入的評審
不 過,如果不在評審工作上傾注一定的時間和精力,這些目標都是無法實現的。僅僅滾動瀏覽下patch,確保縮進正確、所有的變量采取小駱駝拼寫法并 不能構成一次徹底的評審。受到業界的啟發也可以考慮結對編程,這是一個相當流行的做法,但也在所有的開發時間上增加了100%的額外開銷來作為代碼評審工 作的基準。你可能會在代碼評審中花費很多時間,但與結對編程相比,使用的總體工程時間仍少得多。
我認為花在代碼評審工作上的時間應該是原開發時間的25%左右。例如,如果一個開發者花兩天時間實現了個小項目,那么評審者應該花大致4個小時的時間來評審它。
當 然,花在評審工作上多少時間并不是最重要的,只要評審能夠準確無誤的完成即可。特別地,你必須要能理解你正在審查的代碼。這不僅僅意味著你只要懂 該代碼所采用語言的語法即可,它還意味著你必須了解該代碼如何適應于更大的應用環境、組件或庫下。如果你不抓住每一行代碼的全部含義,那么你的評審就不是 非常有價值的。這也是為什么好的評審都不可能非常快的完成:因為還要花時間去調查觸發某個給定函數的不同代碼路徑,要去確保第三方API能夠正確使用(包 括任何邊緣情況),等等。
除了尋找你所審查的代碼中的瑕疵或其它問題之外,你還應該確保:
包含所有必要的測試。
合適的設計文檔已經寫完。
甚至擅長寫測試和文檔的開發人員也并不總能記得在代碼改動之后及時更新。在適當的時候來自代碼評審人員的細微調整對于確保代碼在隨著時間的推移不會變質是至關重要的。
防止代碼評審工作超負荷
如 果你的團隊強制要求做代碼評審,那這是有風險的,因為你的代碼評審工作可能一直積壓,最終到無法管理的地步。如果你兩周之內不做任何評審工作,你 可以很容易的花上幾天時間來趕補它。不過這也意味著當你最終決定去處理它們的時候,你自己的開發工作將遭到一定的意外擱淺。這也使得做好評審工作更加困 難,因為正確的代碼評審需要強烈、持續的腦力勞動,很難這樣數日保持下去。
因此,開發者每天應該竭盡全力的清空他們的評審積壓工作。一個方 法是早晨的第一件事情就用來解決評審工作。在開始自己的開發工作之前先做完所有的優 秀評審工作,你可以防止以后的評審局面失控情況。有些人更喜歡在午休之前或之后或在一天結束后做審查工作。無論你什么時候做這些事情,通過將代碼審查作為 正規的日常工作而不是作為一種分散注意力的工作,你可以避免:
沒有時間處理你的評審積壓工作。
因為你的評審工作還沒做完而延遲項目的發布。
做出一些不再相關的評審,因為在此期間代碼已經改動的非常多。
因為趕在最后一分鐘處理它們而導致評審工作最終完成的很差。
寫易于評審的代碼
無法管理的評審積壓工作也不能全怪評審人員。如果我的同事不管三七二十一的花費一周的時間來給一個大工程項目添加代碼,那么他們發布的patch將真的很難評審,因為在一個階段里有太多的工作要處理,代碼的目的和底層架構體系也會很難理解。
這 是將你的工作切割為一個個可管理單元之所以非常重要的眾多原因之一,我們使用scrum管理方法,所以對我們來說合適的單元是重點。通過一起努 力,用單元來組織我們的工作,并提交僅與我們正在進行的某個單元相關的評審,我們可以寫出更加易于審查的代碼。你的團隊可能使用另一種管理方法,但是原則 都是一樣的。
為了寫出易于評審的代碼,還有一些其它的必備條件。如果要做出一些很棘手的架構決策,為滿足評審者的要求,事先進行討論是合理的。這將使得評審者更加容易的理解你的代碼,因為他們將知道你在代碼中試著達到什么目的以及怎么計劃來達到該目的。這也有助于避免這樣一種情況:在評審者提出一個不同的更好的方法后,你必須要重寫你的大段代碼。
在你的設計文檔里項目架構應該要詳細的描述。這無論如何都是很重要的,因為它能讓一個新的項目成員很快的趕上進度并理解現有的代碼庫。它還能幫助評審者更好的做好自己的工作,這是另一個好處。單元測試也有助于向評審者說明組件應該如何使用。
如果你的patch里包含了第三方代碼,請單獨提交。例如當jQuery的9000行代碼被插入代碼中間時,要做好代碼評審工作就難上加難了。
寫出易于評審的代碼的最重要步驟之一是給你的代碼評審部分添加注釋。 這表示你可以自己瀏覽評審部分,并在任何你 覺得有助于評審者理解代碼意思的地方添加注釋。我發現這樣的注釋僅花費相對較少的時間(經常僅幾分鐘的時間)卻能產生巨大的作用,能讓代碼評審工作完成的 更快、更好。當然,代碼注釋也有許多相同的優點,應該在合適的地方使用,但是通常來說評審注釋更為明智。最后可以說是一個獎勵吧, 研究表明,當開發者重新閱讀和注釋代碼時,竟然發現他們自己的代碼里有很多的瑕疵。
龐大的代碼重構
有時有必要重構能影響許多組件的某個代碼庫。對于一個龐大的應用程序,這個過程可能花費好幾天(甚至更久)且導致龐大的補丁。在這些情況下一個標準的代碼評審工作可能是不切實際的。
最 好的解決方法是遞增式重構代碼。在工作代碼庫的合理范圍內找到能達到你目的的某個改動點。一旦改好了,review通過了,接著進行下一個改動, 直到整個重構工作完成。這個方法可能并不是每次都行得通,但是有想法和計劃,在重構時要避免巨大的補丁通常是實際可行的。像這樣來重構代碼可能要花開發人 員更多的時間,但它同時也產生了更好的代碼質量和更容易的評審工作。
如果真的實現不了遞增式重構代碼(這可能要說一些關于如何寫好和組織好源代碼的事情),一個可能的解決方案是當進行重構工作時用結對編程來代替代碼評審。
解決爭議
你的團隊無疑是由一群聰明的專業人士組成。當大家對某個確定的編碼問題觀點不同時,基本上都會產生爭議。作為一名開發人員,保持開放的心態,在你的評審者更傾向于一個不同的方法時要隨時準備妥協。不要對你的代碼持專有的態度,也不要帶個人評審意見。如果僅僅是因為有人覺得你應該將一些重復的代碼重構為一個可重復利用的函數時,這并不能表明你就不是一個有吸引力的、出色的和有魅力的人。
作為一個評審者,一定要機智。在 改變建議之前,認真考慮下是否你給的提議真的更好或僅僅只是你個人風格問題。如 果你選擇的戰場集中在一些源代碼中明顯需要改進的區域,你將能獲得更多的成功。說一些諸如“考慮下……可能是值得的”或“有人建議……”的話更適合,而不 是“連我的寵物倉鼠都能寫出一個比這更高效的排序算法”。
如果達不到一個中間立場(即雙方都不愿意妥協),那么就邀請一個雙方都尊敬的第三方開發人員過來看看,讓他們給出一些觀點和建議。