干貨分享：六大招教你有效進行代碼 Review
研發同學都知道代碼 Review 的重要性，在騰訊代碼 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
2.1. 第一招：什么時候發起 Review
在代碼 Review 上，Author 需要意識到：Reviewer 的時間是昂貴的。因此在正式邀請 Reviewer 發起代碼 Review 前，Author 有幾項需要注意的點，這些都能提高代碼 Review 的效率，節省 Reviewer 的時間。
2.1.1. MR (Merge Request)
也稱為 PR（Pull Request）， MR 是我們進行代碼 Review 的地方，它記錄著代碼的具體改動，參與者具體的討論過程。好的 MR 應該做到以下幾點：
- 單一：一個 MR 應該只解決一個單一的問題，無論是修復一個 bug，還是實現一個新 feature。Author 應該避免一個 MR 包含不同意圖的代碼改動。單一的 MR 能幫助 Reviewer 快速地了解代碼改動的動機，能有針對性地進行 Review。
- 短小：MR 應該盡量地小，比如一個 feature 引入了較多的改動，需要考慮是否可以拆成獨立的幾塊實現，分開提 MR，比如接口定義、接口實現、邏輯對接等拆分開。
- 詳細: 這里說的詳細是指 MR 應該盡可能地詳細描述它的背景和動機，可以是在 MR 的描述中詳細體現，也可以是連接到具體 issue 或 tapd 單中。需要達到的目的是，其他人翻開一個 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>
大體分三行：
- 【標題行】 必填, 描述主要修改類型和內容。
- 【主題內容】 描述為什么修改, 做了什么樣的修改, 以及這么做的思路等等。
- 【頁腳注釋】 放 Breaking Changes 或 Closed Issues
其中 type 是 Commit 的類型，可以有以下取值：
- feat：新特性
- fix：修改 bug
- refactor：代碼重構
- docs：文檔更新
- style：代碼格式修改
- test：測試用例修改
- chore：其他修改, 比如構建流程, 依賴管理
其中 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 的目的包括：
- 發現那些明顯的疏忽，如代碼 debug 過程中留下的不必要的痕跡，比如 fmt.Println(...)，不小心注釋掉的代碼。
- 之前被 Reviewer 多次提出過的問題。
- Commits 是否正常，在多人協作的情況下 MR 中否帶入了不相關的 Commit。
- Commit Message 是否合適。
Self-Review 是一個非常快速的過程，從我個人的經驗，一般 1-2 分鐘即可完成，所以推薦大家養成 Self-Review 的習慣。
2.2. 第二招：該找誰來 Review
從目的出發，可以從以下幾方面考慮 Reviewer：
- 提高代碼質量。所以首先應該找和代碼改動緊密相關的研發人員參與 Review，比如一起開發某個功能，某個項目，或者一起參與了方案設計討論并給出了有價值意見的研發。
- 獲取意見。找有相關經驗的資深研發幫忙 Review，比如 Java 語言資深的研發、寫過相同或類似系統/功能的研發。
- 形成共識。如果涉及到不同團隊或模塊間的接口改動，或其他會影響其他人的改動，可以邀請相關團隊或模塊的接口人參與 Review，以對改動形成共識。
- 質量把關。對于重要的代碼庫，可能會執行比較嚴格的質量把控，如果設置了必須的 Reviewer，這些 Reviewer 也會參與進來。
- 變動告知。很多情況下一個代碼庫可能只有一個人維護，如果做了些比較特殊的變動，其他人很難發現。因此在做一些重要的但是理解起來不那么直接的地方的時候，最好告知一下相關的研發，以便他們能大概知道發生了什么。
2.3. 第三招：都 Review 些什么
經常會有 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 的時候可以關注以下幾點：
- 變量、方法的命名是否合適，是否真實反映了他們的目的，這方面網上可以找到不少的資料說明。
- 實現的邏輯是否已有現成的庫可以替代。如果有成熟的庫可以使用，盡量不要自己去實現，因為可能會引入不必要的 bug。從我個人的角度，簡潔（大白話就是代碼少）是可讀性一個很重要的指標。
- 關于注釋。代碼注釋不求多，而在于有效，以前也經歷過代碼注釋要求至少達到 30% 以上的年代，現在看來過多依賴注釋其實是對代碼質量的不自信，好的代碼應該盡量做到自解釋。在實際過程中偶爾能看到代碼邏輯實際已經清晰明了，但是用語句不怎么通的英文注釋了一通，最后反而是看懂了代碼才能理解注釋到底說了啥。因此 Review 的時候，不必要的注釋可以建議去掉。
- 不好理解的地方要有恰當的注釋。代碼中如果有特殊處理、特殊的常量、或者不符合一般用法的邏輯需要特別注釋說明一下。
- 代碼的組織。良好的代碼應該有較好的封裝以及層級，使得代碼看起來清晰明了，比如 DAO 層、Service 層。
{ 代碼健壯性 }
- 代碼的改動是否會影響其他功能。
- 用戶參數是否做好細致的校驗。
- 有沒有 Panic 的可能（針對 Go 的說法）。
- 是否會破壞 API 的兼容性。
{ 可擴展性 }
當前的實現方式是否能兼容以后類似的擴展需求。比如在新增接口、API 或者調整參數以解決某一問題上，可以考慮是否后續會有其他類似情況發生。舉幾個例子：
- 假設我們需要定義一個 API 接口去獲取一個用戶的某些信息，例如聯系方式等，我們定義的 API 就不能只返回這些信息，而是應該把用戶相關的信息都返回。
- 假設我要定義一個參數，雖然當前定義成單個元素即可滿足，例如 string, int，但是以后是否會涉及到多個元素的場景，是否定義成 []stirng, []int 是更優的。
這里只是舉了有限的一些例子，在實際 Review 過程中，Reviewer 可以根據自身的經驗從各個角度提出優化的意見。一般需要重點看看：
- 你看不懂或疑惑的地方。
- 打破你常規的地方。
- 復雜的地方。
2.4. 第四招：如何進行
Review 過程中鼓勵 Reviewer 大膽 Comment，有不理解的地方，或者覺得不合適的地方都直接表達出來，Author 對 MR 的 每個 Comment 也要做出反饋，無論是展開討論還是簡單的給個 OK 都是有效的反饋。
Review 的過程可以是：
- Author 在各項確認工作完成后，發起 Review，如果比較急，可以給重要的 Reviewer 發消息請求幫忙 Review。
- Reviewer 看到 MR 后應該先確認 MR 的背景和目的，如果不清楚也無法從 MR 中獲取該信息，最好直接和 Author 溝通。
- Reviewer 直接在 MR 上提出自己的建議或者問題。
- Author 對每個 Comment 進行反饋，并展開必要的討論。
- 復雜的話題可以采用線下討論以提高溝通效率。
- Author 處理完了所有的 Comment，也修改了代碼后，需要在 MR 里 @ 一下相關 Reviewer 告知所有優化已經提交，如果時間比較急也可以直接和 Reviewer 溝通。
- Reviewer 確認沒問題，給 MR 進行 Approve，一般簡單的回復是 LGTM（Lood Good To Me），也可以對 Author 的工作進行贊賞，比如 “God Job” 等。
- Approve 后 MR 由誰來合并這個看自己選擇。
- 如果 Reviewer 提供了很多有用的建議幫助優化代碼，Author 也可以禮貌性地感謝一下 Reviewer。
2.5. 第五招：關于 Comment
代碼 Review 很大程度上就是給代碼挑毛病，而且一般預期挑的越多越好，但代碼是 Author 寫的，很多情況下或多或少會對 Author 造成不適。所以在 Comment 的時候需要盡量注意措辭，尤其是一些主觀的東西，一般以建議的方式給出。當然對于原則性的問題，是要堅守質量標準的，甚至可以直接 Deny 掉 MR。
另外，參與者也不要吝嗇你的溢美之詞，無論是 Author 還是 Reviewer，在 Review 中發現了好的地方或好的建議，請給予對方以肯定和贊美，這樣有助于 Review 有效的進行。
2.6. 第六招：參與者該抱著怎樣的心態
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。
