本文作者:陳德,騰訊後臺開發工程師
研發同學都知道代碼 Review 的重要性,在騰訊代碼 Review 也越來越受大家重視,作為騰訊專有雲平臺研發的一員,我參與了大量的代碼 Review,明顯地感受到有效的代碼 Review 不但能提高代碼的質量,更能促進團隊溝通協作,建立更高的工程質量標準,無論對個人還是團隊都有著重要的價值。本文就為什麼要做代碼 Review 以及如何有效地做代碼 Review 分享一下個人的看法。
為什麼要代碼 Review,相信每個人心中都有比較一致的答案,Google 搜索一下也能找到一大堆的文章。這裡簡單總結幾點:
1)提高代碼質量
這是代碼 Review 的初衷,也是代碼 Review 最直接的價值。Reviewers 根據各自的經驗,思考方式,看問題的角度給代碼提出各種可能的改進意見,從而形成更好的代碼以及產品質量。
我們知道產品問題越晚提出解決它的代價就越大,參與進去的人、要走的流程都會越來越多。代碼 Review 可以說是早期解決問題最有效的途徑之一了,在代碼 Review 中解決掉哪怕一個小問題都能避免後續一堆的麻煩事。
2)形成團隊統一的高質量標準
有效的代碼 Review 最終會在團隊範圍內建立起統一的質量標準,並且會隨著團隊成員的互相學習和促進形成更高的標準。參與者會在代碼 Review 的過程中基於具體問題從不同角度提出改進意見,並最終做出當前最佳的選擇並形成共識。隨著代碼 Review 的有效進行,團隊成員會有意識地關注代碼質量,從而形成越來越高的事實上的質量標準。
3)個人快速成長
通過有效的代碼 Review,Author 和 Reviewer 都將獲得成長,在 Review 過程中參與人就具體的問題展開深入的討論,無論是怎麼寫出整潔的代碼、怎麼更好地更全面地思考問題、怎麼最佳地解決問題,參與人都可以互相取長補短,互相提高。通過具體代碼實現進行的討論往往是最深入和有效的,代碼 Review 是開發者提高代碼能力最重要的途徑之一。
有的人認為代碼 Review 就是 Reviewer 幫助查找代碼中的 Bug,其實代碼 Review 不應該是一個單向的過程,而應該是個雙向交流的過程,Reviewer 幫助 Author 提出代碼改進意見的同時,也是向優秀的 Author 學習的過程。我們都知道提高代碼能力一個有效的途徑是閱讀優秀的項目代碼,但是閱讀項目代碼有著不小的難度,這也是大部分人沒有去執行的原因,而 Review 資深同事的代碼,我們在閱讀代碼的同時能夠直接進行有效的溝通,這是一個快速有效的學習途徑,尤其對開發經驗還不算豐富的開發者而言。
在代碼 Review 上,Author 需要意識到:Reviewer 的時間是昂貴的。因此在正式邀請 Reviewer 發起代碼 Review 前,Author 有幾項需要注意的點,這些都能提高代碼 Review 的效率,節省 Reviewer 的時間。
2.1.1. MR (Merge Request)
也稱為 PR(Pull Request), MR 是我們進行代碼 Review 的地方,它記錄著代碼的具體改動,參與者具體的討論過程。好的 MR 應該做到以下幾點:
2.1.2. Commit Message
之前翻看了不少現存的項目代碼,看到不少的 Commit Message 寫得比較簡單,例如一連串的 "update", "fix",從這些 Commit Message 中完全看不出做了什麼改動,想想如果之後想要定位之前的某個改動,該從哪裡下手。
目前 Commit Message 規範比較常見的有 Angular 團隊的規範,並由此衍生出了 Conventional Commits Specification,可以參照此 Specification 約定 Commit Message 格式規範。
<type>(<scope>): <subject><BLANK LINE><body><BLANK LINE><footer>
大體分三行:
其中 type 是 Commit 的類型,可以有以下取值:
其中 scope 表示的是 Commit 影響的範圍,比如 ui,utils,build 等,是一個可選內容。
其中 subject 是 Commit 的概述,body 是 Commit 的具體內容。
例如:
fix: correct minor typos in codesee the issue for details on typos fixed.Refs #133
Commit Message 可以在 git 中配置模板,這樣可以在 vim 中展示出模板,另外可有工具幫助我們生成和約束 Commit Message,例如 commitizen/cz-cli,這裡不再具體說明。
2.1.3. CI 通過
CI(Continuous Integration),持續集成可以幫助我們自動發現很多代碼中的基本問題,在合適的靜態代碼檢查(lint)配置和良好的單元測試覆蓋下,CI 可以有效地提高代碼的質量。很多人都低估了靜態代碼檢查的能力,實際上現在常見語言的靜態代碼檢查已經能幫助發現不少的 bug 和隱患。對於 Go 語言,可以配置 golangci-lint 來做代碼檢查,單元測試根據實際情況可以制定相應的標準,比如覆蓋率 60%,其中關鍵的代碼邏輯儘量全面覆蓋。
提交代碼 Review 前需要確保 CI 執行通過,這也是為了節省 Reviewer 的時間,能夠通過自動化解決的事情,儘量不要讓 Reviewer 來做,而 Reviewer 發現 CI 未過的 MR 也可以要求 Author 先解決 CI 問題。
2.1.4. Self-Review
我們一般代碼 Review 都是找他人來進行 Review,其實負責任的 Author 在邀請他人來代碼 Review 前也需要自己簡單 Review 一遍,即 Self-Review。
Self-Review 的目的包括:
Self-Review 是一個非常快速的過程,從我個人的經驗,一般 1-2 分鐘即可完成,所以推薦大家養成 Self-Review 的習慣。
從目的出發,可以從以下幾方面考慮 Reviewer:
經常會有 Reviewer 拿到 MR 不知道該 Review 些什麼,其實無論你參與對應項目的深入如何,都可以對代碼進行 Review,也鼓勵不同人從不同的深度、角度去幫助 Review。代碼 Review 沒有固定的形式,它更像是一門藝術,唯一的提高辦法就是實際參與進去。
Review 的時候可以從以下幾個方面入手:
1)簡單的 Review
在 CI 通過的情況下,最簡單的 Review 方式可能只需要這樣:
Reviewer:在實際環境中都驗證過了嗎?
Author:當然驗證過了
Reviewer:LGTM
這是一種提醒式的 Review。確認一句:是否在環境中驗證過了,或者進一步把能想到的重要的驗收點提出來確認一遍。即使是這種最簡單的 Review 實際上也是有價值的,我們很難保證所有研發都會在提 MR 前實際在環境中驗證自己所做的修改,也很難保證單元測試、e2e 測試能 Cover 住所有的情況,Reviewer 基本上也不可能都自己去環境上跑一遍。讓 Author 去確認實際上就是提醒 Author 去確保改動至少是真實有效的,尤其是對一些已發布版本的 Bugfix,一定要提醒實際自測通過。
類似的提醒還包括:相關的文檔(外部的)是否相應更新了、這個改動是否會有兼容性的問題、性能是否有影響。他們的本質就是提醒 Author 自己去思考他們可能遺漏的問題。
2)常規的 Review
代碼 Review 一般都會從代碼風格、變量命名、語法統一處入手,當然這些應該更多的藉助於 CI 等自動化手段來保證,但是在相關流程還不是很完善的前提下還是有必要進行關注。
此外代碼可讀性、代碼健壯性、代碼可擴展性都是 Review 時關注的點。每一個關注的點都依賴於 Reviewer 的實際經驗,這裡只簡單舉幾個例子:
{ 代碼可讀性 }
代碼是寫給人看的,因為沒有不需要維護的代碼,無論是 Author 自己後續維護代碼,還是他人接手代碼,能快速地理解代碼邏輯是非常必要的。
代碼 Review 的時候可以關注以下幾點:
{ 代碼健壯性 }
{ 可擴展性 }
當前的實現方式是否能兼容以後類似的擴展需求。比如在新增接口、API 或者調整參數以解決某一問題上,可以考慮是否後續會有其他類似情況發生。舉幾個例子:
這裡只是舉了有限的一些例子,在實際 Review 過程中,Reviewer 可以根據自身的經驗從各個角度提出優化的意見。一般需要重點看看:
Review 過程中鼓勵 Reviewer 大膽 Comment,有不理解的地方,或者覺得不合適的地方都直接表達出來,Author 對 MR 的 每個 Comment 也要做出反饋,無論是展開討論還是簡單的給個 OK 都是有效的反饋。
Review 的過程可以是:
代碼 Review 很大程度上就是給代碼挑毛病,而且一般預期挑的越多越好,但代碼是 Author 寫的,很多情況下或多或少會對 Author 造成不適。所以在 Comment 的時候需要儘量注意措辭,尤其是一些主觀的東西,一般以建議的方式給出。當然對於原則性的問題,是要堅守質量標準的,甚至可以直接 Deny 掉 MR。
另外,參與者也不要吝嗇你的溢美之詞,無論是 Author 還是 Reviewer,在 Review 中發現了好的地方或好的建議,請給予對方以肯定和讚美,這樣有助於 Review 有效的進行。
2.6.1. Author
首先需要明確一點,是 Author 自己對代碼的質量負責,因此應當懷著感恩的心去看待堅持認真幫你 Review 代碼的 Reviewer,因為並不是所有你加的 Reviewer 都有時間和精力來幫你提高代碼質量。
也正是因為 Author 是自己代碼的 owner,所以不要依賴於 Reviewer 去幫你守代碼質量的大門,像 「代碼 Review 的時候你怎麼就沒看出來」,「這不是你建議我這麼做的嗎」 這樣的話千萬別說。Reviewer 只是幫你提高代碼質量,因此我們該做的工作都要去做,比如細緻的 Reviewer 可能某些情況下直接提出了代碼優化的建議,Comment 時貼上了優化的代碼片段,Author 不能直接假設它一定是能正常工作的,而應該對它進行完整的驗證。
對 Reviewer 給出的 Comment,不要有牴觸的情緒,對你覺得不合理的建議,可以委婉地進行拒絕,或者詳細說明自己的看法以及這麼做的原因。有時候一個你覺得不合理的建議可能代表一個新的思考角度,也可能代表 Reviewer 自身代碼能力上的不足,無論是哪個,無論最終是 Author 還是 Reviewer 得到了提高,都應該是好事。
2.6.2. Reviewer
Review 代碼既是幫助提高代碼質量的過程,也是 Reviewer 提高自己代碼能力和溝通能力的過程。因此應該在 Review 的同時保持一個學習者的心態,既要發現對方代碼中的缺陷,也要努力去發現代碼中的亮點。切忌單純以挑毛病的心態去 Review 代碼。
有不少 Reviewer 在寫 Comment 的時候會猶豫,擔心自己提出的問題或建議比較「蠢」,因此猶猶豫豫下看完了代碼抱著一堆疑慮最終啥也沒留下。其實在代碼 Review 的時候大可不必有什麼心裡負擔,有什麼疑惑的、不清楚的地方或者有什麼自己的想法,可以直接提出來,有時候一個簡單的 Comment 就可能會激起 Author 和你的 Comment 毫不相干的新思路。再者你即使沒幫 Author 提高代碼,讓 Author 幫你提高思考不也是件不錯的事情嗎。
Reviewer 也不需要去關注自己的「產出」,並不是一定要提出一堆問題才是好的代碼 Review,Author 代碼寫得很棒也是很正常的,如果從你的角度覺得代碼沒問題,大膽給個 LGTM 甚至是 Approve。