CodeReview的時候，每個人都會關(guān)心最佳實踐，但最壞的實踐有時可能會更有啟示意義。
CodeReview是研發(fā)團隊必不可少的，但并不總是正確的。這篇文章指出了所有開發(fā)者在CodeReview時或提交拉取請求時可能都會遇到的一些常見的錯誤模式，并對這些錯誤模式進行了總結(jié)：
錯誤模式：挑毛病
想象一下下面的場景。代碼作者花了幾個小時，甚至幾天的時間來創(chuàng)建他們認為最有效的解決方案。他們考慮了多種設(shè)計方案，并選擇了看起來最相關(guān)的路徑。他們考慮了現(xiàn)有應(yīng)用程序的架構(gòu)，并在適當?shù)牡胤竭M行了修改。然后，他們將自己的解決方案以拉動請求的形式提交，或者開始了代CodeReview過程，他們收到的專家反饋是：
- "你應(yīng)該使用標簽，而不是空格。"
- "我不喜歡這部分的大括號在哪里。"
- "你的文件末尾沒有空行。"
- "你們的詞庫是大寫的，應(yīng)該用句子大寫。"
雖然新的代碼要與現(xiàn)有代碼的風(fēng)格保持一致是很重要的，但這些東西幾乎不需要人工審核員來完成。人工審核員的成本很高，而且可以完成計算機無法完成的事情。檢查是否符合風(fēng)格標準是計算機可以輕松完成的事情，這就分散了代碼審核的真正目的。
如果開發(fā)人員在代碼審核過程中看到很多這樣的注釋，說明這個團隊要么是沒有風(fēng)格指南，要么是有了風(fēng)格指南，但檢查風(fēng)格還沒有實現(xiàn)自動化。解決的辦法是使用checkstyle等工具來確保風(fēng)格指南已經(jīng)被遵循，或者使用sonarqube來識別常見的質(zhì)量和安全問題。而不是依靠人工審核員來警告這類問題，持續(xù)集成環(huán)境可以做到這一點。
有時，如果沒有代碼指南，或者內(nèi)部代碼風(fēng)格隨著時間的推移而變化，在不同的部分有不同的風(fēng)格，那么這種自動檢查可能會很困難。在這種情況下，有一些方法可以應(yīng)用自動檢查。例如，一個團隊可以同意做一個單一的提交，應(yīng)用約定的代碼風(fēng)格，并且不包含其他的更改。或者一個團隊可以約定，當一個文件因為bug或功能而被更改時，該文件也會被更新到新的樣式，而自動工具可以被配置為只檢查更改過的文件。
如果一個團隊有多種代碼樣式，而它沒有辦法自動檢查樣式，也容易落入下一個陷阱。
錯誤模式：不一致的反饋
每一個被邀請審閱代碼的開發(fā)者，至少要多邀請一個意見，而且可能更多。每個人都可以同時持有不止一種意見。有時，CodeReview可能會陷入審查者之間關(guān)于不同方法的爭論，比如說是使用流還是經(jīng)典的for循環(huán)最好。如果團隊成員對同一段代碼有不同的意見，那么開發(fā)人員應(yīng)該如何進行修改，結(jié)束審閱，并將代碼推送到生產(chǎn)中？
即使是一個審稿人的想法也很容易發(fā)生變化，可能是在一次審稿中，也可能是在一系列的審稿中。在一次審閱中，一個審閱者可能會催促作者確保使用O(1)讀操作的數(shù)據(jù)結(jié)構(gòu)，而在下一次審閱中，審閱者可能會問為什么不同的用例會有幾個數(shù)據(jù)結(jié)構(gòu)，并建議通過單一結(jié)構(gòu)進行線性搜索來簡化代碼。
當一個團隊對自己的"最佳實踐"是什么樣子的沒有一個明確的想法，當團隊還沒有弄清楚自己的優(yōu)先級是什么的時候，這種情況就會出現(xiàn)：
- 代碼是否應(yīng)該向著更現(xiàn)代的Java風(fēng)格發(fā)展？還是更重要的是代碼的一致性，因此，繼續(xù)到處使用"經(jīng)典"構(gòu)造？
- 在系統(tǒng)的所有部分中，對數(shù)據(jù)結(jié)構(gòu)進行O(1)讀操作是否重要？還是有些部分的O(n)可以接受？
幾乎所有的設(shè)計問題都可以用"這要看情況"來回答。為了對答案有一個更好的想法，開發(fā)人員需要了解他們的應(yīng)用和團隊的優(yōu)先級。
錯誤模式：最后一分鐘的設(shè)計變更
開發(fā)者在CodeReview過程中最讓人士氣低落的反饋是：當評審者從根本上不同意方案的設(shè)計或架構(gòu)，并強行完全重寫代碼時，要么通過一系列的評審來逐步完成（見下一節(jié)），要么粗暴地拒絕代碼，讓作者重新開始。
CodeReview不是評審設(shè)計的正確時機。如果團隊按照經(jīng)典的"網(wǎng)關(guān)式"CodeReview，那么在最后一步讓另一個開發(fā)人員看代碼之前，代碼應(yīng)該是可以工作的，所有的測試都應(yīng)該是通過的。在這一點上，幾個小時、幾天，甚至可能是幾周（雖然我真的希望不是幾周；CodeReview應(yīng)該是小事一樁，但這是另一個話題）的努力已經(jīng)花在了被審查的代碼上。在CodeReview中指出底層設(shè)計是錯誤的，這是在浪費大家的時間。
CodeReview可以作為設(shè)計審查，但如果這是CodeReview的意圖，那么審查應(yīng)該在實現(xiàn)之初就進行。然后，在開發(fā)人員還沒有走得太遠之前，他們可以把自己的想法勾勒出來，也許會有一些存根類和方法，以及一些有意義的名稱和步驟的測試，也許還可以提交一些文字或圖表，以便讓團隊成員對將要采取的方法進行反饋。
如果團隊成員在關(guān)口審查中發(fā)現(xiàn)了真正的展示性設(shè)計問題（也就是說，當代碼完成并運行時），團隊應(yīng)該更新流程，以便更早地定位這些問題。這可能意味著要做其他類型的審查，比如上一段中建議的審查，白板上的想法，配對編程，或者與技術(shù)負責(zé)人討論建議的解決方案。在最后的CodeReview中發(fā)現(xiàn)設(shè)計問題是對開發(fā)時間的浪費，也是對代碼作者的極大打擊。
錯誤模式：乒乓球Reviews
在一個理想的世界里，作者會提交代碼進行評審，評審人員會提出一些明確的解決方案的意見，作者提出修改建議并重新提交代碼，評審結(jié)束，代碼就會被推送。但如果這樣的事情經(jīng)常發(fā)生，誰還能說得清CodeReview的過程是有道理的呢？
在現(xiàn)實生活中，經(jīng)常出現(xiàn)的情況是這樣的：
- 一個CodeReview開始了。
- 一些審稿人提出了幾個建議：有的小而容易，有的蓬頭垢面，沒有明顯的解決方案，有的復(fù)雜。
- 作者做了一些修改：至少是簡單的修改，或者說是幾處修改，力求讓審稿人滿意。作者可能會向?qū)徃迦颂岢鰡栴}來澄清一些事情，或者作者可能會提出意見，解釋為什么沒有做出特定的修改。
- 審稿人回來后，接受一些更新，對其他的修改提出進一步的意見，找到他們不喜歡的地方，回答問題，并在審稿中與其他審稿人或作者爭論他們的意見。
- 代碼作者做更多的修改，增加更多的評論和問題，以此類推。
- 審稿人檢查修改，提出更多的意見和建議，以此類推。
- 步驟5和6重復(fù)進行，或許永遠都是這樣。
在這個過程中，理論上來說，修改和批注應(yīng)該向著零的方向下降，直到代碼準備好為止。最郁悶的情況是，每一次迭代都會帶來至少和已經(jīng)結(jié)束的舊問題一樣多的新問題。在這種情況下，團隊就進入了"CodeReview的無限循環(huán)"。發(fā)生這種情況的原因有很多：
- 如果審稿人吹毛求疵，如果審稿人給出的反饋不一致，就會出現(xiàn)這種情況。對于陷入這些習(xí)慣的審稿人來說，有無限多的問題需要找出，有無限多的意見需要提出。
- 當審稿時沒有明確的審稿目的，或者審稿時沒有準則可循，就會出現(xiàn)這種情況，因為這樣一來，每個審稿人都會覺得每一個可能出現(xiàn)的問題都必須找出來。
- 當不清楚審稿人的評論對代碼作者的要求是什么時就會發(fā)生。是不是每一條評論都意味著必須要進行修改？所有的問題是否都暗示著代碼沒有自證，需要改進？還是有些評論僅僅是為了教育代碼作者下一次，而提出問題只是為了幫助審稿人理解和學(xué)習(xí)？
評論應(yīng)該被理解為阻止者或不是阻止者，如果審稿人決定代碼需要修改，他們需要明確說明代碼作者應(yīng)該采取什么行動。
同樣重要的是，要明白由誰來決定審核是否"完成"。這可以通過任務(wù)清單上的檢查項目來實現(xiàn)，也可以由個人授權(quán)說"足夠好"來完成。通常需要有一個人能夠打破僵局，解決分歧。這個人可能是高級開發(fā)人員、領(lǐng)導(dǎo)或者是架構(gòu)師，甚至是團隊中的代碼作者，因為在團隊中，他們之間的信任度很高。但是，在某些時候，需要有人說"評審結(jié)束了"或者"當這些步驟完成后，評審就結(jié)束了。"
錯誤模式：幽靈審查
在這里我承認我最容易犯的反常的地方：幽靈化。無論我是審閱者還是代碼作者，在代碼審閱中都會出現(xiàn)一個點（有時就在開始的時候！），在審閱過程中，我根本就沒有回應(yīng)。也許有一個重要或有趣的功能被要求我審閱，所以我決定把它留到"更好的時候"，等我可以"真正好好看一看"的時候再做。又或許是Review的量大，我想留出充足的時間。又或許是我是作者，在迭代（或二十次）后，我就是無法面對閱讀和回復(fù)評論了，所以我決定等"等我的腦袋想好了再來"。
聽起來是不是很熟悉？
不管是什么原因，有時在審查過程中有人根本沒有反應(yīng)。這可能意味著在這個人看完代碼之前，審查工作就已經(jīng)死氣沉沉了。這是一種浪費。即使有人在創(chuàng)建一個資產(chǎn)（新代碼）上投入了時間，但在它投入生產(chǎn)之前，它并沒有增加價值。事實上，當它在代碼庫中越來越落后于其他代碼庫的時候，它很可能已經(jīng)腐爛了。
有幾個因素會導(dǎo)致幽靈審查。龐大的代碼審核量是一個因素，因為誰愿意去翻閱幾十個或幾百個修改過的文件？不重視CodeReview是另一個因素，因為不重視CodeReview是真正的工作或交付成果的一部分。困難的或令人沮喪的CodeReview經(jīng)歷是另一個主要因素。沒有人愿意停止編碼（開發(fā)人員通常喜歡的東西），去參加一項耗費時間和破壞靈魂的活動。
以下是解決幽靈審查的建議：
- 確保CodeReview的規(guī)模要小。每個團隊都要制定出自己的定義，但這將是幾個小時或幾天的復(fù)審工作，而不是幾周的時間。
- 確保CodeReview的目的很明確，審查人員應(yīng)該找的東西很清楚。當范圍是"找到代碼中可能存在的任何問題"時，很難激勵自己去做一件事。"
- 在開發(fā)過程中留出時間來做CodeReview。
最后一點可能需要團隊的紀律性，或者團隊可能希望通過（例如）通過目標或任何用來決定開發(fā)人員的生產(chǎn)力的機制來獎勵良好的CodeReview行為來鼓勵允許時間。
你的團隊能做什么？
對于研發(fā)團隊，專注于創(chuàng)建一個行之有效的CodeReview流程。我在我的博客上寫過這方面的內(nèi)容，但想在這里分享一下這個過程的一部分：
- 在進行CodeReview時，有很多事情需要考慮，如果開發(fā)人員在每次CodeReview時都擔(dān)心所有的事情，那么任何代碼都幾乎不可能通過評審流程。要實現(xiàn)一個適合所有人的CodeReview流程，最好的方法是考慮以下問題。
- 團隊為什么要做審閱？當有一個明確的目的時，審查員的工作會更容易，代碼作者也會從審查過程中減少討厭的驚喜。
- 團隊成員要找的是什么？當有了目的，開發(fā)人員可以在審閱代碼時創(chuàng)建一套更有針對性的東西來檢查。
- 誰來參與？誰來做評審，誰負責(zé)解決意見沖突，誰最終決定代碼是否合格？
- 團隊何時進行復(fù)審，復(fù)審何時完成？審核可以在開發(fā)人員在代碼工作的時候迭代進行，也可以在流程結(jié)束時進行。如果沒有明確的指導(dǎo)，一個評審可能會一直進行下去，如果沒有明確的指導(dǎo)，代碼最終什么時候可以進行。
- 團隊在哪里做評審？CodeReview不需要特定的工具，所以審查可能就像作者在辦公桌上帶領(lǐng)同事看他們的代碼一樣簡單。
一旦回答了這些問題，你的團隊就應(yīng)該能夠創(chuàng)建一個運行良好的CodeReview流程。記住：CodeReview的目的應(yīng)該是讓代碼投入生產(chǎn)，而不是證明開發(fā)人員有多聰明。
結(jié)論
CodeReview的錯誤模式可以通過建立一個明確的CodeReview流程來消除，或者至少是緩解。許多團隊認為他們應(yīng)該進行CodeReview，但他們沒有明確的準則，為什么要進行CodeReview。
不同的團隊需要不同類型的CodeReview，就像不同的應(yīng)用程序有不同的業(yè)務(wù)和性能要求一樣。第一步是弄清楚團隊為什么需要審閱代碼，然后團隊就可以著手于：
- 自動化的簡易檢查（例如，檢查代碼樣式，識別常見的BUG，發(fā)現(xiàn)安全問題）。
- 就審查的時間、審查的內(nèi)容以及審查結(jié)束后由誰決定等問題制定明確的準則
- 將CodeReview作為開發(fā)過程的一個關(guān)鍵工作內(nèi)容
專注于為什么要進行CodeReview，將幫助團隊創(chuàng)建CodeReview流程的最佳實踐，這樣就更容易避免CodeReview的錯誤模式。