技術團隊如何高效落地代碼CR
?引言
代碼CR(Code Review)是軟件研發活動中保障平臺產品質量的重要環節,相信很多技術團隊平常都會進行代碼CR。就拿阿里來說,一般周二和周四都是發布日,那么在發布上線某項功能之前都要組織進行發布代碼CR,CR不通過的代碼必須修改檢查通過后才能發布上線,可見一線互聯網大廠技術團隊對于代碼CR的重視程度。雖然大家對于代碼CR都不陌生,但是在自己團隊中實際落地的時候不免還是會遇到這樣或者那樣的問題,比較典型的問題有如下幾種:
1、到底是所有的代碼都需要進行CR,還是只要核心業務代碼才需要進行CR?
2、怎樣才能讓reviwer能夠認真評審代碼?
3、線上CR還是線下CR?
4、代碼CR很費時間和精力,如何才能保證在花費的時間和精力后可以達到預期效果?
本文就和大家探討下到底怎么做才能在技術團隊中高效的落地代碼CR活動,而不至于最后導致代碼評審活動流于形式只是走個流程而已。
為什么要進行代碼CR
提升團隊代碼水平
一般技術團隊都會由不同技術水平以及不同工作年限的同學組成,而Code Review是非常好的大家在一起互相學習代碼設計的機會,因為在Code Reiew過程中不僅僅會檢查業務代碼邏輯問題,還包含了結構設計、程序性能、代碼安全、程序魯棒性等等方面綜合性檢查。因此團隊中的核心骨干如果能夠重度參與代碼CR活動中,不僅在一定程度上可以幫助其他同學的成長,同時也可以協助新同學快速融入團隊。

另外在編碼的時候,代碼CR機制會像一只無形的手,不斷鞭策著團隊成員不要寫爛代碼。因為大家知道自己寫的代碼需要在團隊內部進行公開的CR,這樣壓力就會油然而生,它會促進自己不要寫爛代碼,因為每個人也不希望自己寫的爛代碼曝光在大家面前,這樣自己面子上也掛不住。因此CR機制在一定程度上可以讓團隊同學避免寫一些短期收益的代碼,從而欠下技術債留給未來維護同學。

CodeReview不是人情世故,而是程序員的技術煉金場。
?確保設計實現一致
在程序猿的日常工作中,通常在業務方需求KO之后會進行對應需求的設計和實現。但是在實際的研發活動中,經常會出現實現和設計之間存在一定的偏差gap,而這些gap可能會導致后期上線的Bug以及代碼維護問題,因此在代碼CR的時候就要重點關注設計與代碼實際實現之間存在差異問題,尤其是需求Owner要重點review業務-》設計》實現的一致性。

統一團隊編碼規范
在實際的代碼CR過程中,經驗豐富的老司機分別會從命名、代碼結構設計、程序性能等方面進行分析。其實同樣一個需求,讓100個程序員來做,寫出來的代碼可能有100種樣子,我覺得這很正常。實現邏輯可以五花八門,但是在一些比較通用的編碼規范層面,需要保持統一。
如果有這樣的業務場景,你定義了一個訂單的DTO類,其中有個字段是訂單的審核狀態,假設有待審核、審核通過、審核不通過三種狀態,我們通過定義枚舉的方式來表示不同的狀態。這里的inspectStatus分別對應枚舉中的三個狀態屬性。大家覺得下面的代碼有什么問題?
public class OrderDTO {
...
/**
*審核狀態
*/
private Integer inspectStatus
...
}
實際上并不是代碼本身有什么問題(都是屬性能有啥問題),而是在可讀性方面存在問題。假設你是剛接手負責這個模塊,當你看到這個定義的時候,是不是迫切想知道到底這個審核狀態有哪幾種,但是由于代碼不熟悉,你并不知道去哪里尋找,因此在可讀性以及可維護性方面不盡如人意。
遇到這種情況我們可以在審核狀態字段的注釋上面加上一個@link,可以直接鏈接到對應的狀態枚舉類,這樣后來維護業務的同事可以通過實體類直接查看到訂單的各個審核狀態,總比自己無頭蒼蠅的在工程中找或者問其他組內同事來的效率高。因此我們在編寫代碼的時候不僅要考量如何實現當前的需求,也要想著如果未來別人來維護我寫的代碼,那么怎樣才能讓后續維護的同學更好更快的掌握業務邏輯。
public class OrderDTO {
...
/**
*審核狀態
*{@link com.mufeng.eshop.biz.order.InspectStatusEnum}
*/
private Integer inspectStatus
...
}
這里舉了個看似簡單的例子,但是在實際的編碼中卻十分常見,因此需要對團隊中的代碼規范進行統一的規定。代碼規范性層面可以參考《阿里巴巴Java開發手冊》,另外Idea有對應的插件Alibaba Java Coding Guidelines。
厘清業務邏輯細節
一般一個技術團隊可能會負責多條不同的業務線。這些業務可能都是存在一定的關聯關系的。但是平時同學們都在忙于應付各種各樣的業務方需求,大家可能沒有太關注同組同學所負責的業務的具體邏輯細節,因此通過代碼CR可以讓大家有機會了解各個業務線的邏輯細節,這樣更加便于團隊成員厘清上下游的業務邏輯,將來涉及到完整業務鏈業務需求的時候,在進行設計的時候可以考慮的更加全面以及識別一些關鍵設計點。
如何保證代碼CR效果
如果我們想要保證代碼CR的落地效果,我們就需要搞清楚到底哪些因素會影響技術團隊代碼CR效果。這里大致總結了日常工作中影響代碼CR效果的因素:
對于提交代碼評審的同學:
1、不清楚提交代碼CR的范圍;
2、不清楚需要給哪些人提交代碼CR;
3、怎樣才能讓別人認真評審代碼;
對于評審別人代碼的同學:
1、不清楚需要CR代碼的業務上下文是怎樣的,不容易判斷代碼結構設計的合理性;
2、一下子提交幾千行代碼,哪些代碼是CR的重點內容,哪些不是;
上述問題都是制約技術團隊代碼CR落地效果最常見的問題,我們到底應該怎么解決這額問題呢?
線上評審結合線下評審
線上評審一般是主要的代碼CR方式,但是在提交評審的時候還是要遵循一定的原則,以便于提高代碼評審的效率。
1、每次提交CR的代碼不能過多,如果每次評審的時候一下子推給別人幾千行代碼,估計對應的reviwer看都不想看,很難保證review的質量;
2、在提交評審的時候,需要附上一定的說明,闡述清楚這些代碼主要實現什么樣的業務需求,主要核心邏輯在哪些文件中,這樣reviwer在評審代碼的時候可以有的放矢。
線下評審作為線上評審的重要組成部分,比如一周一到兩次的線下會議評審一般可以滿足需要。在線上評審之前最好先和reviwer敲定好時間以及需要評審的代碼,提前準備好需要CR的代碼背景,比如說對應具體的需求是什么,自己在進行代碼落地的時候是如何分析問題的,大致的類結構是如何設計的等等,這樣reviewer們可以比較清晰的理解代碼的背景。
另外線下評審的代碼量不要過大,時間盡量保持在一個小時左右。評審會議的時候要記錄大家提出來的建議以及問題,會后修改后再和大家對焦確認。
找到合適的代碼評審者
將代碼提交給合適的Reviewer進行評審這一點非常重要,因為如果將代碼提交給了沒什么業務關聯的或者和自己技術水平差不多的Reviewer,一方面業務不相關的同學很難理解其中的業務規則代碼看起來也費時又費力,另一方面水平相似無法高屋建瓴的提出來改進意見,因此基本很難獲得比較好的review結果。
1、向團隊中經驗豐富的程序員提交CR,以便于獲得更加高水平的代碼設計反饋;
2、向業務需求Owner提交CR,需求Owner一般對于這部分的業務需求非常熟悉,因此可以從業務層面或者技術層面給出更好的意見;
3、向修改過相同文件的同學提交CR,這樣大家彼此知道自己的修改意圖以及原因,便于評估影響范圍。
建立評審獎勵機制
或許是因為大家平時工作太忙,或許是因為提交給了不合適的reviewer。我們總是擔心別人到底有沒有認真CR我們的代碼,那我們到底該怎么激發大家認真review代碼呢?這里提供一個可落地實操的機制,比如我們可以在團隊內部建立明確的代碼CR獎勵機制,對于在代碼CR過程中評審出來的高質量Bug的reviewer進行獎勵(獎牌或者獎杯都可以,同時在P8下面的大團隊中進行通報表揚提升影響力)。每個月評選捉蟲高手,注重數量更加注重質量,通過這樣的獎勵機制來正向引導大家去積極進行代碼CR。
不過這里面存在一個隱含的Bug,就是如果團隊中有一個技術大牛,那么大家可能都會把代碼提交給他來審核,那么技術大牛代碼評審量就會非常大,對于大牛來說就變成甜蜜的負擔了,因此還是要有所取舍,比較核心重要的代碼再提交給技術大牛進行評審,這樣既保證了核心業務邏輯代碼的評審權威性,也不會給團隊中資深工程師的工作負擔。






















