天天看點

【轉】Google 官方文章——如何去做code reviewcr的标準在cr中要看些什麼如何浏覽CLreview的速度如何寫review評論譯者總結

Google 前幾天公開了一篇谷歌的工程實踐文檔。而且文檔的内容都是跟 code review 相關的内容,裡面包含了 Google 工程師如何進行 code review 的内容,以及 code review 指南。 原文位址: google.github.io/eng-practic…

本文的名詞解釋:

  • cr: code review
  • cl: change list,指這次改動
  • reviewer: cr的那個review人
  • nit: 全稱nitpick,意思是雞蛋裡挑骨頭
  • 作者: 也就是本次CL的開發者,原文中是以author來稱開發者的。以老外的思維,意思是“CL的作者”

cr的标準

cr(Code review)主要目的在于確定Google 的代碼庫代碼品質越來越好。而所有相關的工具與流程皆是因應這個目的而生。為達到此目的,勢必需要做出一連串的權衡與取舍

首先,開發人員必須能夠在自己負責的任務上有所進展。如果你從來沒有向代碼庫送出改進過的代碼,那麼代碼庫就永遠不會改進。另外,如果一個reviewer使cr都很難進行的話,那麼開發人員就不願意在将來進行改進。

另一方面,reviewer有責任確定每個change list(下稱CL)的品質,保證其代碼庫的整體代碼品質不會越來越差。這可能很棘手,因為通常會随着時間的推移,代碼需要降級才能讓代碼運作起來,特别是當團隊受到嚴重的時間限制時,大家覺得必須走捷徑才能實作他們的目标。

此外,reviewer對他們正在review的代碼擁有所有權和責任。他們希望確定代碼保持一緻、可維護,以及下文的“在cr中可以得到什麼”中提到的内容。

是以,我們有以下規則,作為我們在cr中所期望的标準:

一般來說,當CL存在的時候,reviewer應該贊成它,此時它肯定會提高正在工作的系統的整體代碼品質,即使這個CL并不完美。這是所有cr中的首要原則。當然,這是有局限性的。例如,如果一個CL添加了一個reviewer不希望在其系統中使用的功能,那麼reviewer當然可以拒絕,即使代碼設計得很好。

這裡的一個關鍵點是,沒有“完美”的代碼,隻有更好的代碼。reviewer不應該要求作者在approve之前對一篇文章的每一小段進行潤色。相反,reviewer應該權衡發展的需要和他們所建議的change的重要性。reviewer不應追求完美,而應追求持續改進。作為一個整體,提高系統的可維護性、可讀性和可了解性的CL不應該因為它不是“完美的”而被延遲幾天或幾周。

reviewer應該随時可以留下評論,表達一些更好的東西,但如果不是很重要,可以在評論前加上“nit:”這樣的字首,讓作者知道這隻是一個他們可以選擇忽略的修飾點。

指導

cr有一個重要的功能,教開發人員一些關于語言、架構或一般軟體設計原則的新知識。留下有助于開發人員學習新知識的評論是可以的。随着時間的推移,共享知識是提高系統代碼健康度的一部分。你隻需要記住,如果你的評論純粹是教育性的,但對達到本文檔中描述的标準并不重要,請在其前面加上“nit:”,或者以其他方式表明作者不必在本CL中解決它。

原則

現實和資料推翻了個人喜好。在代碼風格方面,風格指南(style guide)是絕對的權威。任何不在style guide中的一些點(如空格等)都是個人偏好的問題。代碼風格應該與現有的一緻。如果項目沒有以前的統一風格,那就接受作者的風格。

軟體設計的各個方面從來都不僅僅是一個純粹的代碼風格問題或者是個人喜好問題。它們是以基本原則為基礎的,應當以這些原則為依據,而不僅僅是以個人意見為依據,有時幾乎都沒有選擇的。如果作者能夠證明(通過資料或基于原理的一些事實)他的方法是同樣有效的,那麼reviewer應該接受作者的偏好。否則,代碼風格選擇取決于軟體設計的标準原則。

如果沒有其他規則适用,那麼reviewer可以要求作者與目前代碼庫中的内容保持一緻,隻要這些代碼不會惡化系統的整體代碼健康狀況。

解決沖突

在cr的任何沖突中,第一步應該始終是開發人員和reviewer根據本文和《CL Author’s Guide》,嘗試達成共識。

當達成共識變得特别困難時,reviewer和作者需要進行面對面會議,而不是僅僅試圖通過cr的注釋來解決沖突。(不過,如果這樣做,請確定将讨論結果記錄在CL的評論中,以供将來的讀者閱讀。)

如果這不能解決問題,最常見的解決方法就是更新。通常情況下,更新的途徑是進行更廣泛的團隊讨論,讓team leader參與進來,請求代碼維護人員做出決定,或者請求技術經理提供幫助。不要因為作者和審稿人不能達成一緻意見而讓一個其他人袖手旁觀。

在cr中要看些什麼

注意:在考慮每一點時,一定要考慮cr的标準。

設計

在cr中重要的是看CL的總體設計。CL中不同代碼段的互動是否有意義?此更改屬于你的業務代碼庫還是屬于引進來的其他代碼庫?它是否與系統的其他部分很好地內建?現在是添加此功能的合适時機嗎?

功能

這個CL做了開發者想要的嗎?開發者對這些代碼的設計初衷使用者有好處嗎?“使用者”通常既是最終使用者(當他們受到更改的影響時)又是開發人員(他們将來必須“使用”此代碼)。

大多數情況下,我們希望開發人員在進行cr時能夠對CL進行充分的測試,使其能夠正常工作。但是,作為reviewer,仍然應該考慮邊緣狀況,尋找問題,嘗試像使用者一樣思考,并確定僅通過閱讀代碼就不會看到錯誤。

如果你願意的話,你可以自己去驗證CL。如果改動會直接帶來的使用者可見的影響,比如說ui改動,驗證CL的變化是非常關鍵的。 在閱讀代碼時,很難了解某些更改會對使用者産生怎樣的影響。對于這樣的更改,如果不友善自己測試,則可以讓開發人員示範該功能(demo)。

另外,在cr期間考慮功能性特别重要的點是,cl中是否并發式程式設計,理論上可能導緻死鎖或競争條件。這些類型的問題很難通過運作代碼來發現,通常需要有人(開發人員和reviewer)仔細考慮,以確定不會引入問題。(注意,這也是不使用平行式程式設計的一個很好的理由,在這種情況下,可能出現競争條件或死鎖,這會使代碼檢查或了解代碼變得非常複雜。)

複雜性

一個CL是否複雜到超過預期的必須?針對任何層級的CL必須确認這幾點:單行代碼是否過于複雜?函數是否過于複雜?class是否過于複雜?“複雜”通常意味着該代碼很難閱讀,很有可能也意味着其他開發者在修改這段代碼的時候引入其他bug

其中一種複雜性就是過度設計(Over-engineering)造成的,開發者會讓那段代碼過度通用化,超過了原本所需,或者還新增了系統目前不需要的功能。reviewer應特别注意一下過度設計。鼓勵開發者解決他們知道現在需要解決的問題,而不是推測将來可能需要解決的問題。當那些将來出現的問題出現的時候才開始解決它們,因為那時候你可以更加清晰看見問題的原樣子

Donald Knuth說過:過早的優化是萬惡之源(Premature optimization is the root of all evil)

測試

請将單元測試、整合測試、端到端測試視為要求CL所做的适當變更。一般CL内除了生産環境的業務代碼外,測試也應該要被加入其中。除非該CL是為了處理某個緊急事情而存在。

另外,也要確定測試是正确、合理、有用的。測試并非來測試它們本身,一般也極少為了測試而測試(如測試一下測試代碼有沒有問題又走了測試流程),是以我們要保證測試是有效的。

當代碼真的有問題,測試是否會失敗?如果被測試的程式發生改動時,測試是否會産生誤報?每一個測試是否做出了簡單而有用的斷言?不同的測試方法之間的測試是否适當分開?

請記住,測試代碼也是必須維護的代碼,不要因為它們不在主要關注的範圍内。

命名

開發者是否為了每一個東西都挑了一個适當的名字?一個好的命名意味着,通過名字就足以完整表達該東西的作用是什麼或者要做什麼。但是同時名字也不要長得難以閱讀

推薦參考文章「Clean code 101 — Meaningful names and functions」

注釋

開發者是否用可了解的英文留下清晰的注釋? 這些注釋是否真的必要?

通常注釋是解析這段代碼為什麼存在的時候是相當有用的,而不應該去解釋某段代碼正在做什麼。如果代碼本身不能解釋清楚的話,意味着它更加需要簡化了。當然也有例外,比如解釋正規的表達式或者複雜的算法正在做什麼的時候,注釋解釋這段代碼正在做什麼就相當有用。但對于大部分注釋來說是用來說明那些不包含在程式本身但資訊,比如說為什麼要這樣子做的理由

檢視該CL之前的注釋也很有幫助,或許有一個todo項目現在可以一處、一個評論建議為什麼不要進行這種更改等等。

要注意的是,注釋與class、module、function的檔案不同。後三者要能夠表達一段代碼的目的、如何使用它、使用時行為

風格

Google 對于主要語言都有提供風格指南(style guide),甚至包括大多數冷門語言,是以要確定CL遵循适當的指南上的說明。

如果你想改進CL中某些不包含在風格指南中的要點時,請在評論前加上Nit: ,讓開發人員知道這是你認為可以改善代碼的小問題且并非強制性的。但記住,不要僅根據個人風格偏好阻止送出CL。

開發者不應該在 CL内同時包含主要風格的改動與其他代碼的修改,這樣會導緻難以看出CL到底做出什麼改動。同時也會讓合并和復原更為複雜,并産生其他問題。例如,如果作者想要重新格式化代碼的話,讓他們将新格式在一個新CL裡面重構。

文檔

如果CL更改了建構、測試、互動、釋出的時候,請檢查是否也同時更新相關文檔, 包括README,g3doc 頁面和其他生成(generated) 的參考檔案。如果CL 删除或棄用 了一些代碼,請考慮是否應該删除對應文檔,如果缺少文檔時請詢問。

每一行代碼

仔細review配置設定給你的每一行代碼。有些東西,比如資料檔案(data files)、生成的代碼(generated code)、大型資料結構(large data structures),你可以稍微掃過。千萬不要在掃過開發者寫的 class、函數、代碼區塊 時,去假設它内部是沒問題的。 很顯然的某些代碼需要比其他代碼更仔細的review。這是必須由你做出的判斷,但至少你應該确定你了解所有代碼在做些什麼。

如果閱讀代碼過于複雜并且減慢review速度時,那麼你再繼續review前,要讓開發者知道這件事,并等待他們為這段代碼做出解釋、說清楚。在Google 我們聘請許多優秀的軟體工程師,而你也是其中的一員。如果連你也無法了解的話,很可能其他人也不會。是以,你要求開發者去說清楚這段代碼時,同時也在幫助未來的開發人員了解這些代碼。

如果你能夠了解,但覺得沒有資格進行某部分的稽核,請確定 reviewer中有一個适合(合格) 的人來review該部分。尤其是針對安全性、并發性、可通路性、國際化等複雜問題時。

上下文

在充足的上下文下檢視CL通常很有幫助。一般來說,cr工具隻會顯示修改部分周圍的幾行代碼而已。但有時你必須檢視整個檔案以確定改動是否合理。比方說,你可能隻看到添加4行新代碼,但實際上檢視整個檔案時會發現這4行是被加在有50行的代碼裡,此時需要将它拆解為更小的函數。

以整個系統的角度出發來思考CL也是很有用的。該CL是否改善整體系統的代碼品質,亦或是會讓整個系統更加複雜?是否缺少測試?千萬不要接受會降低整體系統的代碼品質的CL。因為大多數系統是由于許多小改動的積累而變得複雜,是以阻止新的改動引入複雜性(盡管很小)也非常重要。

優點

當你在CL裡看到優點時記得告訴開發者,尤其是當他們用很棒的方式來解決你的評論時。cr通常隻關注存在的錯誤,但它們也應該同時應該為良好實踐提供鼓勵與欣賞。這點在指導開發者時尤其重要:與其告訴他們做錯什麼,還不如告訴他們做對了什麼。

個人認為并非不要指出錯誤,而是多以鼓勵來代替指出錯誤,讓其他開發者更有動力想将事情做好。其實透過簡單的一句話讓對方知道哪裡做得很好,未來他們會将繼續保持下去,并為其他開發者帶來的正面影響

【轉】Google 官方文章——如何去做code reviewcr的标準在cr中要看些什麼如何浏覽CLreview的速度如何寫review評論譯者總結

标明每個commit 修改什麼,可以幫助reviewer快速進入狀況

【轉】Google 官方文章——如何去做code reviewcr的标準在cr中要看些什麼如何浏覽CLreview的速度如何寫review評論譯者總結

此時就不要吝啬你的稱贊了!

總結

在cr時,請務必確定:

  • 代碼經過完善的設計
  • 功能性對于使用者們是好的
  • 對于任何UI改動要合理且好看
  • 任何并行程式設計的實作是安全
  • 代碼不應該複雜超過原本所必須的
  • 開發者不該實作一個現在不用而未來可能需要的功能
  • 代碼有适當的單元測試
  • 測試經過完善的設計
  • 開發者對于每樣東西有使用清晰、明了的命名
  • 注釋要清楚且有用,并隻用來解釋why而非what
  • 代碼有适當的寫下檔案(一般在g3doc)
  • 代碼風格符合style guide
  • 確定你檢視被要求review的每一行代碼、确認上下文、確定你正在改善代碼品質,并贊揚開發人員所做的好事與優點吧!

如何浏覽CL

現在你已經知道review時要看些什麼,但怎樣才是review分散在多個檔案中的改動最有效的方法?

  1. 改動是否合理?他是否有好的描述(description)
  2. 優先看CL 最重要的改動。它整體是否有完善的設計?
  3. 用合理的順序看CL 剩餘的改動
步驟1: 用宏觀的角度來看待改動,檢視CL描述以及它做什麼

該改動是否有意義、合理?如果在第一時間認為不應該發生這種變化,請立即說明為什麼不該這樣做的原因。當拒絕類似這樣的更改時,向開發人員提供建議告訴他們應該怎麼做什麼也是一個好主意。

例如,你可以說:“看起來你已經完成一些不錯的事情,謝謝!但我們實際上正朝着删除你正在修改的FooWidget系統的方向前進,是以現階段我們不想對它進行任何新的修改。 不如重構我們的新BarWidget class如何?”

需要注意的是,reviewer在委婉拒絕該CL的同時也要提供替代方案,而且仍然保持禮貌。這種禮貌是很重要,因為我們希望表明即使不同意也會互相保持尊重。

如果你有幾個CL裡包含你不想這樣做的改動時,你應該重新考慮開發團隊的開發過程,或釋出開發流程給外部貢獻者知道,以便在任何CL被撰寫前有更多的溝通。最好是在他們開始投入前說“不”,避免那些已經投入心血的工作現在必須被抛棄或徹底重寫。

提供替代方案讓對方知道該怎麼做,而非讓其自行獨自摸索。

【轉】Google 官方文章——如何去做code reviewcr的标準在cr中要看些什麼如何浏覽CLreview的速度如何寫review評論譯者總結

指出問題,告知替代方案或指點方向

步驟2: 檢查CL主要的部分

找到CL最核心的部分的那些檔案。通常CL内會有檔案包含大量的邏輯改動,而它正是CL的主要部分。是以我們要首先檢視這些主要部分。這有助于為CL的其他較小部分提供适當上下文,而且這樣通常可以提高review速度。如果CL太大導緻于無法确定哪裡是主要部分時,請向開發者詢問首先應當檢視的内容,或者要求他們将CL拆分為多個CL。

如果在主要部分發現存在一些主要的設計問題時,即使沒有時間立即檢視CL的其餘部分,也應立即留下評論告知此問題。因為事實上,因為該設計問題足夠嚴重的話,繼續review其他部分的代碼可能隻是浪費時間,因為其他正在審查的其他代碼可能都将無關緊或消失。

立刻發送關于主要設計的評論非常重要有兩個主要原因:

  • 通常開發者在送出CL後,在等待review過程中便已經開始着手基于該CL的新工作。此時如果正在review的CL存在重大設計問題的話,開發者将不得不重新寫所有基于它的後續所有CL。是以你要在他們有問題的設計上投入之前阻止他們。
  • 主要設計變更通常需要較長時間才能完成,但每個開發人員幾乎都有自己deadline。為了趕上deadline并保證代碼品質,開發者需要盡快開始或重做CL 任何的主要設計變更。
步驟3: 用合理的順序看CL 其餘的改動

一旦确認整個CL沒有重大的設計問題時,試着找出一個有邏輯順序來review剩餘檔案,并確定不會錯過其中任何一個。通常在浏覽主要部分後,最簡單的方式是按照cr工具提供的順序來浏覽每個檔案。有時在閱讀主要代碼前先閱讀測試也是非常有幫助的,如此一來你就可以了解應該做、看些什麼。

review的速度

為什麼review速度要快

在Google我們優化開發團隊共同生産産品的速度,而不是優化個人開發的速度。個人的開發速度很重要,但它不如整個團隊的開發速度重要。當cr很慢時,會發生以下幾件事:

  • 團隊整體的速度下降。review慢的話,對于團隊其他人來說重要的新功能與缺陷修複将會被延遲數天、數周甚至數月,隻因為它們正在或者等待review。
  • 開發人員開始抗議cr。若reviewer每隔幾天才回應一次,但每次都要求對CL進行重大更改的話,那麼開發人員可能會感到非常沮喪與覺得困難,而這通常會演變成抱怨。如果reviewer請求相同的實質性更改(且确實可以改善代碼品質狀況),但在每次開發人員送出新的改動時都能快速反應的話,這些抱怨往往會消失。大多數關于cr的抱怨,往往可以通過加快流程來解決。
  • 代碼品質會受到影響。當review很慢時,會造成開發者送出不完全盡如人意的CL 的壓力越來越大。評論的越慢也會阻止他人進行代碼清理、重構、甚至是對現有CL 的進一步改進。

cr應該要多快?

如果你并沒有處于需要專注工作的時候,那麼你應該在CL被送出後盡快進行review。review回複最長的極限是一個工作日。若遵循以上指南,意味着一般的CL應該在一天内得到多輪review(如果必要的話)。

速度vs中斷

但有時候個人的速度優先度會勝過團隊速度。如果你處于需要專注工作的時候(比方說寫代碼),不要打斷自己去做cr。

研究證明,若開發者在被打斷後會需要很長時間才能恢複到原本順暢的開發流程。是以,開發的時候,打斷自己實際上會比讓另一位開發者等待review來得更加昂貴。

取而代之的是,我們可以在投入到處理他人給的review評論之前,找個适當的時機點來進行cr。這有可能是當你的目前開發任務完成後、午餐、剛從會議脫身或從微型廚房回來等等。

快速回應

當我們談論cr的速度時,我們關注的是回應時間,而非CL需要多長時間才能完成并送出。在理想情況下,整個過程應該是快速的。

總的來說個人回應評論的速度,比起讓整個cr過程快速結束來得更為重要。即使有時需要很長時間才能完成整個流程,但若在整個過程中能快速獲得來自reviewer的回應,這将會大大減輕開發人員對于緩慢的cr過程的挫敗感。

如果真的忙到難以抽身而無法對CL進行全面review時,你依然可以快速的回應讓開發者知道你什麼時候會開始稽核、建議其他能夠更快回複reviewer,又或者提供一些初步的廣泛評論。(注意:這并不意味着你應該中斷開發去回複——請找到适當的中斷時間點去做)

很重要的是,reviewer員要花足夠的時間來進行review,確定他們給出的LGTM,意味着“此代碼符合我們的标準”。

盡管如此,理想的個人的回應速度還是越快越好。

跨時區review

在面對時區不同的問題時,盡量在他們還在辦公室時回複作者。如果他們已經回家了,那麼請確定在他們第二天回到辦公室前完成。

LGTM 評論

為加快速度,在某些情況下reviewer可以給予LGTM或Approval,即便CL上仍有未解決的評論。類似情況會發生在:

  • reviewer确信開發人員會适當地處理所有剩餘的評論
  • 剩餘的評論是微不足道的或它們不需由開發者來處理
  • reviewer必須清楚指明他們是指上面哪種情況

LGTM 評論對雙方處于不同的時區時尤其值得考慮,否則開發人員會等待一整天才得到“LGTM,Approval”。

大型改動

如果有人要求reivew時,但由于改動過于龐大導緻你難以确定何時才有時間review它時,你通常該做的是要求開發人員将CL拆解成多個較小的CL,而不是一次review巨大的CL。這種事是可能發生的,而且對于reviewer非常有幫助,即便它需要開發人員付出額外人力來處理。

如果CL無法分解為較小的CL,且你沒有足夠時間快速檢視整個CL内容時,那麼至少對它的整體設計寫些評論,并發送回開發人員以便進行改進。身為reviewer,你的目标之一是在不犧牲代碼品質的狀況下,避免阻檔開發人員進度,或讓他們能夠快速采取其他更進一步的動作。

cr的能力會随着時間進步

如果你遵循這些準則,并且對于cr非常嚴格的話,後面你會發現整個cr流程會越來越快。因為開發者學到什麼是品質好的代碼,并于開頭就送出一個很棒的CL,而這正恰好隻需要越來越少的時間。而reviewer則學會快速回應,而非在過程中加入不必要的延期。 但不要為提高想象中的速度,而對cr标準和代碼品質做出妥協,畢竟從長遠來看它實際上并不會讓任何事情發生得更快。

緊急狀況

在某些緊急情況下,CL會希望放寬标準以求迅速地通過整個cr過程。但請看什麼是緊急情況來知道哪些情況實際上屬于緊急情況,而哪些不符合。

如何寫review評論

如何面對被推遲處理的評論

有時開發人員會推遲處理cr産生的評論。要麼他們不同意你的建議,要麼他們會抱怨你太嚴格了。

誰是對的

當開發人員不同意你的建議時,請先花點時間考慮一下他們是否是正确的。因為通常他們比你更了解代碼,是以他們可能真的比起你來說對代碼的某些層面具有更好的洞察力。他們的論點有意義嗎?從代碼品質的角度來看它是否合理?如果是的話,讓他們知道他們是對的,然後讓問題沉下去。

但開發人員也并不總是對的。在這種情況下,reviewer應該更進一步解釋為什麼認為自己的建議是正确的。一個好的解釋通常展示了“對開發人員的回覆的了解”以及有關“為什麼被要求對出做出改動”等資訊。尤其是當reviewer認為給出的建議會改善代碼品質時,便應該繼續宣揚自己的論點。隻要他們相信所需的額外的工作量最終會改善整體代碼品質。提高整體代碼品質這件事,往往是經由每個微小的改動來發生。有時往往需要幾番解釋一個建議才能讓對方真正了解你的用意。切記,始終保持禮貌,讓開發人員知道你有聽到他們在說什麼,隻是你不同意該論點而已。

讓開發者沮喪

reviewer有時會認為若自己堅持改進的話,會讓開發人員覺得沮喪不安。的确開發人員有時會感到很沮喪,但這通常是十分短暫的,甚至後來他們非常感謝你幫助他們提高代碼品質。一般來說,隻要你在評論中表現得很有禮貌,開發人員實際上根本不會感到沮喪,這些擔憂都僅存在于reviewer心中而已。沮喪很多時候是對于cr評論的寫作方式有關,而并非來自reviewer對于代碼品質的堅持。

晚點再來整理幹淨

一個常見的推遲原因是開發人員希望完成任務(這可以了解)。他們不想通過另一輪cr來準許這個CL。此時他們通常會說在以後的CL進行整理,是以你現在應該要給LGTM。當然部分開發人員非常擅長這一點,并且立即送出一個修複問題的後續CL (follow-up CL),但根據過往經驗,這種後續“清理行為”非常少見。除非開發者在CL獲得準許之後立刻進行清理動作,否則這事根本不可能發生。這不是因為開發人員不負責任,而是因為他們可能有很多其他工作要完成,于是清理工作便會在成堆的工作中被遺忘。是以,通常最好堅持開發人員在代碼在合并後清理它們。因為讓人們「稍後清理」是緻使代碼庫品質狀況下降最常見的狀況。

如果CL引入了新的複雜性的話,除非是緊急情況,否則必須在送出之前将其處理掉。如果CL導緻暴露周圍的問題且現在無法解決它們的話,開發人員應該将缺陷記錄下來并配置設定給自己,避免後續被遺忘。又或者他們可以選擇在代碼中留下TODO的注釋并連結到剛記錄下的缺陷。

關于review嚴格性的常見抱怨

如果你先前以相當寬松的标準并轉趨嚴格的進行cr的話,一些開發人員會開始大聲地抱怨。一般來說,提高review的速度會讓這些抱怨逐漸消失。這些抱怨可能需要數個月才會消失,但最終開發人員會看到嚴格的review所帶來的價值,因為嚴格的review幫助他們産生的優秀代碼。而且一旦發生某些事情時,最大聲的抗議者甚至可能會成為你最堅定的支援者,因為他們會看到變得review變嚴格後所帶來的價值。

解決沖突

如果你遵循前面所有操作,但仍然遇到無法解決的雙方之間的沖突時,請參考前面的cr的标準以擷取有助于解決沖突的準則和原則。

譯者總結

cr的标準

  • reviewer 有責任保證CL的品質,作為reivew的代碼的owner
  • reviewer不應追求完美,而應追求持續改進
  • cr具有指導意義
  • 代碼風格應該與現有的一緻。如果項目沒有統一風格,那就接受作者的風格
  • 解決沖突難以達成共識時,需要面對面或者拉起更大的團隊讨論,帶上leader

在cr中要看些什麼

  • CL的總體設計
  • 功能驗證,功能性對于使用者們是好的,對于任何UI改動要合理且好看
  • 是不是很複雜,有沒有過度設計
  • 代碼有适當的單元測試
  • 測試經過完善的設計
  • 規範命名,看見名字就知道是什麼
  • 合适的注釋,注釋應該是why而不是what
  • 代碼風格遵循style guide,如果需要改代碼風格應該在另一個CL解決
  • CL更改了建構、測試、互動、釋出的時候,也要更新文檔
  • 仔細review每一行代碼(除了資源檔案、生成的代碼、大型資料結構)。如果比較複雜得讓開發者解釋
  • 安全性、并發性、可通路性、國際化等複雜問題時需要更合适的人來review
  • 以整個系統的角度出發來思考CL
  • review的時候,與其告訴開發者做錯什麼,還不如告訴他們做對了什麼

如何浏覽CL

  • 用宏觀的角度來看待改動,檢視CL描述以及它做什麼
  • 檢查CL主要的部分
  • 用合理的順序看CL 其餘的改動

review的速度

  • review速度慢會導緻團隊整體的速度下降、開發人員開始抗議cr、代碼品質會受到影響
  • 如果你處于需要專注工作的時候(比方說寫代碼),不要打斷自己去做cr。
  • 個人回應評論的速度,比起讓整個cr過程快速結束來得更為重要
  • 在面對時區不同的問題時,盡量在他們還在辦公室時回複作者
  • 為加快速度,在某些情況下reviewer可以給予LGTM或Approval,即便CL上仍有未解決的評論
  • 由于改動過大導緻難以review時,通常該做的是要求開發人員将CL拆解成多個較小的CL
  • cr的速度應該要越來越快,但不要為提高想象中的速度,而對cr标準和代碼品質做出妥協

如何寫review評論

  • 當開發人員不同意你的建議時,思考一下誰是正确的,并解釋清楚,保持禮貌。
  • 如果review堅持己見,會讓開發者沮喪。沮喪很多時候是對于cr評論的寫作方式有關,并非來自reviewer對于代碼品質的堅持。
  • 如果CL引入了新的問題的話,除非是緊急情況,否則必須在送出之前将其處理掉。
  • 如果現在無法解決review的評論的問題的話,TODO的注釋并連結到剛記錄下的缺陷。

review太嚴格被抱怨在怎麼辦

提高review的速度會讓這些抱怨逐漸消失。這些抱怨可能需要數個月才消失,但最終開發人員會看到嚴格的review所帶來的價值,因為嚴格的review幫助他們産生的優秀代碼。

繼續閱讀