開發與維運

如何讓評審人愛上我

眾所周知,每個有技術追求的團隊,都試圖想把Code Review做到極致。正所謂——未經審視的代碼,不值一提。為什麼Code Review如此重要,本篇不再贅述,僅做簡單總結:

  • 提早發現問題,防患故障於未然
  • 快速提高團隊新同學的編程能力

然而夢想很豐滿,現實很骨感,團隊的評審活躍度和評審質量往往令人堪憂,緣由種種,上至技術TL、下至代碼功能的實現者,甚至產品經理(嗯,一定是業務太重導致沒時間做Code Review)都能成為做不好Code Review的罪魁禍首。今天小編作為一個開發者,放下外部的客觀因素,僅從一個代碼的實現者,一個被評審人的角度去思考如何讓評審變得高效而富有意義。換句話說:如何讓評審人愛上我(被評審人)。

為什麼要讓評審人愛上我


在以往的文章討論中,我們往往更專注於如何提升評審人的水平,如何找出評審代碼的問題,或者如何讓被評審人能快速解決提出的問題。但我們忽略了評審人的感受,似乎評審人就是一個毫無感情的找BUG機器。當然,不排除一些自動化掃描的工具(比如:Codeup的缺陷檢查、漏洞分析等等),它們確實是AI機器人。但除此以外很多時候因為對代碼的業務理解、團隊的編碼風格等問題,還是需要團隊的其他成員作為評審者介入其中。對於不走心的敷衍評審我們撇開不談,評審者參與一次評審還是挺耗費精力和時間的,需要時間去閱讀、理解、並指出疑問。因此,我們應該放下防備,坦然對待評審過程,並對評審人心懷感激。

如何讓評審人愛上我


既然我們已經下定決心與評審人來一場甜蜜的Code Review之旅,怎樣才能讓評審人愛上我(的評審)?我從一次評審的全週期來分析,分別為:

  • 【評審前】如何做好評審前的準備;
  • 【評審中】如何對待評審中的討論/問題;
  • 【評審後】評審後需要做什麼。

提交評審前


重點:
  • 認真審視自己的代碼
  • 做好提交(Commit)
  • 保證測試基線

認真審視自己的代碼

在評審中,最忌諱的一點就是犯一些低級錯誤,這樣不僅會給評審者很差的印象,也會將評審者的時間浪費在一些低級錯誤之中。所以,我們要在提交代碼前自己先認真審視自己的代碼。

格式問題

格式問題,常發生在一些初建的團隊,大家有不同的開發習慣、編碼風格,在單人開發中,並沒有什麼問題。當這個項目涉及到多人協作時,就會顯得很麻煩。比如下面這個評審,其實是沒有實際的改動只是編碼風格的格式化。但密密麻麻的文件變動,往往會讓人心神不寧。
因此在提交前,我們應按照團隊的規範,設置好自己的編輯器格式工具,並在提交前檢查是否存在格式上問題,避免這種問題在Code Review的時候浪費大家的時間。

拼寫錯誤

拼寫錯誤是屬於低級錯誤了,目前主流的編輯器都支持拼寫檢查,只要大家認真對待編輯器高亮的warning,就能找出來啦。

1.png

代碼規範掃描


2.png

一些非常基礎問題,可以通過自動化工具掃描出來。比如針對java的開發規約檢測,代碼平臺提供的漏洞檢測、安全掃描等。在提交以後看到分支上改動的commit都是綠色的pass,心情也會好很多。畢竟咱給評審者提供的代碼也是經過一遍初選的複賽選手了。

3.png

做好提交(Commit)

說到提交(commit),很多同學都覺得沒什麼。不就是git commit 麼。或者commit的時候,因為Git的強制要求必須包含message,才隨便輸幾個只有自己懂的提交說明,然後git push拉倒。比如下面這種提交說明,我在很多兄弟團隊的代碼倉庫都有見過。

4.png

這種提交說明的問題,我就不再贅述了。接過老項目、給老工程填過坑的人都懂。因此在評審階段,我們就要做好提交。

5.png

  • 要點一:按功能拆分提交,拒絕大Diff評審

在平時的開發中,大家往往都是多個任務並行開發。很多同學又不喜歡來回切換變更的分支,導致很多評審都是多個功能一股腦的提交,因此評審自然也就變得很大。或者在開發一個比較大的需求時,沒能將功能進行拆分細化,導致所有的改動都在同一個評審中,甚至在同一個Code Review中。例如:

6.png

這個評審,中間混雜了數十個功能的提交,評審者很容易迷失在不同功能的邏輯迷宮中。更有甚者,會提交增刪行數高達上萬行的評審。對於這種超大評審,我不太相信評審者會認認真真地看完。有研究自媒體分享的統計,現代人的注意力只能聚焦5分鐘,如果一個分享超過5分鐘無法結束,人們往往會因為想不起來前面的內容而放棄。評審也是如此,長達數萬行的改動,都在同一個評審中,即便是在評審頁面上提供鉚釘已閱讀的文件的功能,也很難在短時間內容理解上下文的邏輯。因此評審者需要耗費長達半個小時以上的獨立時間來閱讀評審的改動,而這對於正常的開發人員是非常崩潰的。因此,我們在完成一個功能的時候,應當合理的按功能拆分,將提交變小(small is beaut iful)。當改動過於龐大應該考慮拆分多個評審進行。

7.png

8.png

9.png

  • 要點二:描述清楚本次提交的內容、改動點、改動影響

一個評審的開始,從打開頁面到評審開始,一般眼睛的順序是:評審標題->評審描述->改動列表->改動詳情

評審的標題和評審的描述往往就是我們提交的內容生成的。而作為評審開始的前兩個關鍵點,評審提交描述是非常重要的。比如下面這種例子:

10.png

我相信各位作為評審人,看完這個評審已經想關掉評審了(一些暴躁老哥甚至直接點拒絕)。令人費解的評審標題,空白的評審描述。即便我們拿著Code Review坐在評審者的電腦前、或者釘釘上附上評審解釋,這種體驗都是非常不好的,因為大家都不願意注意力被分散。就像我們寫代碼一樣,大家都更願意在開發過程中聚焦在編輯器上,而不是在不同的屏幕間來回切換(查看需求、被人中途打擾等)。因此我們應該寫好描述,儘量讓評審開始的時候,大家都能在一塊屏幕(一個頁面上)完成評審的所有工作。

除此之外,良好提交說明,可以提前讓評審者感知到改動點和改動影響。比如下面:

11.png

評審者從描述中就能確定我們改動的地方和改動的影響,從而可讓評審者更快的進入狀態,而不需要自己去閱讀詳細內容猜我們改了什麼。

  • 要點三:功能改動和非功能改動儘量分開

在一次功能中,我們除了功能性的改動,可能還包含一些非功能性的變動,如果都揉和在一起,往往看起來非常的難受。比如下面這種情況:

12.png

在這個改動中,我除了做一些功能上改動,還順便修改了文檔、對以往的格式問題做了格式化。雖然都是同一個改動,但觀感還是非常差的。就像老師講課,好的老師一定是按內容歸納,按內容教授,而不是代數幾何混在一起講。

因此我們可以在評審的提交中做進一步的拆分。還是針對上面的例子。我們調整後的樣子為:

13.png

在評審頁面的左側點擊全部提交,通過提交信息選擇我們需要獨立評審的提交內容。

14.png

選擇功能性改動的提交,進行功能改動的評審。

15.png

選擇非功能性提交,便可以只關注非功能的改動。比如這裡我們只用單獨看下README裡的改動就可以,全程清爽。

保證測試基線

在很多已經成型的項目中,往往都有充分的測試覆蓋掃描。當我們提交代碼的時候,在功能上我們第一個要保證的就是以往的測試都能通過(除非是這個測試用例已經廢棄或存在錯誤,這種我們應該單拎一個功能來修正),這也是對評審者的尊重,因為連功能的正確性都無法保證的評審是無意義的。

16.png

在雲效中,我們可以將評審的目標分支作為保護分支,在自動化檢查中配置自動化檢查。通過在流水線中通過靈活的配置來設置我們工程中需要卡點檢查。

17.png

然後,在我們提交的評審的時候,就會自動觸發我們配置的卡點了。如果卡點不通過,是不允許評審通過的。因此我們在提交評審中,要注意卡點的內容,要讓我們的評審“綠”起來。

18.png

評審中


重點:
  • 積極對待評審反饋
  • 代碼是最好的解釋
  • 原諒評審人的誤解/失誤
  • 保證Code Review的時效性

積極對待評審反饋

一部分同學反感代碼評審的原因是,他們認為評審人是有意為難自己。不能排除在一些比較複雜的社會環境中確實可能存在這種問題。但是作為被評審人,我們還是要積極的看待評審的反饋。畢竟雞蛋裡挑骨頭的前提是要有骨頭,問題提出就一定有其合理性。絕大部分技術人對待代碼還是中立的,不能積極面對評審更多的是我們擔心自己的代碼不夠好,怕別人指出問題。但換個角度想,評審人其實也是在幫助我們,早點在評審階段發現問題,總比線上出了問題要好。另外,在不斷的評審中,我們也能快速的進步自己的編碼能力。

代碼是最好的解釋

當評審者對我們的代碼提出疑問的時候,我們除了解釋,更應該做的是考慮為什麼會出現這種疑問。有時候雖然我對評審人的疑問做了詳細的回覆,評審人似乎也理解了。但這個評審真的就結束了嗎?馬克思老人家說,我們應該透過現象看本質。畢竟評審人可能不只一個,即便當前的其他評審人看到這個解釋也能明白,但我們無法保證後續涉及這塊的代碼改動再次評審的時候,會有人能明白這裡的歧義。因此這種存在歧義的評審質疑,其本身的問題是代碼未能做很好的解釋。

面對這種情況,一般的建議添加充分的代碼註釋。但我更建議我們直接對代碼進行重構,讓代碼本身就能解釋清楚其意義。

原諒評審人的誤解/失誤

孔子云:人非聖賢孰能無過。評審者在評審中也會存在對正確代碼的誤解。在這種時候,我們不能得理不饒人,應當保持謙遜。正如在開篇中提到的,評審者實際是對我們提供幫助的,幫消滅我們自己未知的隱患。因此我們不應該貿然反擊,應當理性迴應,在解釋的同時也是對我們自己的代碼的一個很好的回顧。除此之外,我們還應意識到這裡另一個問題——  存在即合理 ,如果評審者提出了疑問,是否證明這裡存在不合理性,可能是代碼有歧義需要重構?

保證Code Review的時效性

評審也應該有時效性。因為評審人評審的代碼,往往不是自己的參與的業務,評審週期拉的太長,可能會想不起來。因此在評審中,往往會設置一個提醒的鉤子。配合釘釘的webhook接收,能儘早的感知評審的人的意見。

19.png

在修改結束後及時給予反饋。這樣讓評審者對評審還沒“冷卻”的時候,就能更快的繼續評審,大家就能早早下班啦。

20.png

21.png

評審後


評審後,評審的內容會保存在平臺上,後面可以隨時查看回顧。除此之外,在評審後最重要的是選擇一個合併方式。一般我會選擇squash的模式進行合併。將評審中的所有提交合併為一個commit。這樣合併到主幹中,一個提交就是一個功能。注意,這個和前文的提交拆分並不是一回事。在提交評審階段,我們為了方便評審,將提交拆為功能改動和非功能改動。但在合併階段因為已經通過了評審。我們要將一個功能的改動放到一個提交中。

22.png

點擊合併後,評審中的多次提交最終會壓縮為一個commit合併到目標分支中。這樣也就保證了一個功能一個commit的原則。在後續排查問題和線上回滾都會非常方便。

23.png

P.S. 合併方式應該依據各自團隊對工程管理和開發模式的選擇來決定,以上只是一個簡單的例子。

總結


總結一下,如果讓評審者愛上給我做評審,我會這麼做:

  1. 提交前認真審視自己的代碼,拒絕低級錯誤
  2. 按功能拆分提交,寫好提交說明
  3. 讓所有的單側、集測變綠通過,保證質量基線
  4. 積極對待評審反饋
  5. 用代碼解釋評審者的疑問
  6. 正確對待評審者的誤解/失誤
  7. 及時修正/反饋,保證評審的時效性
  8. 評審後選擇正確的合併方式

本文作者:陳博俊,阿里雲技術專家,Git開源項目貢獻者,負責阿里巴巴經濟體代碼平臺及雲效代碼平臺底層架構設計及運維開發。


內外開屏.png

長按識別上圖二維碼 立即報名

6大專場

27位海內外技術大咖

邀你共享效能盛宴

鎖定6月23日-6月24日

9:00-12:00  14:00-18:00

我們期待與您共建研發效能美好未來!

Leave a Reply

Your email address will not be published. Required fields are marked *