天天看點
傳回

【分享】 codeReview 的重要性

【分享】 codeReview 的重要性
研發都知道代碼 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 幫你提高思考不也是建不錯的事情嗎。

代碼品質 代碼能力 項目代碼
上一篇: ROS學習之catkin_make
下一篇: 【架構學習分享】HttpRunner

繼續閱讀