Code Review 在丁香醫生前端團隊的實踐

丁香園F2E發表於2018-09-12
View前端

時間過得很快，轉眼間 Code Review 機制在丁香醫生前端團隊已經運作一年多了。今年4月初時，將團隊在 Code Review 方面的一些經驗在丁香園前端團隊進行了分享，各個業務線的前端同學們逐步開始嘗試 Code Review 機制，目前也有了一定的收穫。是時候將這些實踐經驗落實到文字上，來和更多的朋友們進行交流了。

起因

世上沒有無緣無故的愛，也沒有無緣無故的恨。同樣，也沒有無緣無故的 Code Review。最開始時，丁香醫生前端有2個人，基本上是1人在做丁香醫生 SPA 專案，1人做丁香醫生管理後臺專案。

將時間點放到17年初，團隊從2個人變為了3個人，此時主要有三個前端專案（丁香醫生 SPA、丁香醫生管理後臺以及丁香醫生 Hybrid App）在迭代，其中主要是 SPA 專案會涉及到三個人的交叉維護。這個階段便會開始暴露出一些小問題。比如：

  • 編碼風格不一致
  • 有些他人寫的業務邏輯，在交叉維護時，需要花更多的時間上手
  • 一些低階的 bug 在程式碼部署到測試環境才被發現

為了解決這些問題，我們決定開始嘗試 Code Review。專案的程式碼是託管在公司內網的 Gitlab 上的，於是我們會開始摸索著基於 GitLab 中專案的 Merge Request 進行他人程式碼的 Code Review。

17年 Q2 時，我們開始頻繁的迭代丁香醫生小程式，同時運營團隊也會開始提出一些運營類H5的需求。團隊成員有4人了。隨著新鮮血液的加入，我們遇到了新的問題：

  • 新人的加入提高了團隊程式碼風格的差異性
  • 在不是很瞭解現有專案的基礎上，實現的新功能程式碼會產生冗餘
  • 誰來為新成員的程式碼質量和成長負責？（注意：這是重要的一點）

此時我們依舊在做 Code Review，但實際上並沒有嚴格的去執行，也沒有一個關於 Code Review 的標準供大家遵守。

毫無疑問的一點：隨著丁香醫生業務的發展，這些問題是需要被解決的，否則長遠來看無論是對於團隊還是團隊成員，都是有較大傷害的。

17 年 Q3 時，團隊已經有 6 個人了。每加入一個新人，上述問題的複雜度就會增加一些。為了解決這些問題，團隊決定將 Code Review 作為一項基本制度，嚴格去執行。

如何去做 Code Review？

前提

在開始嚴格的去做 Code Review 之前，我們確定了三點基礎規範。

  1. 基於專案版本控制，統一專案遵守的 Git 分支模型
  2. 對於 JavaScript，使用統一的 Eslint 規則
  3. 結合團隊成員現有風格，明確統一的程式碼規範

工具

使用的工具就地取材，依舊是 GitLab。整個 Code Review 流程在 GitLab 專案中有兩個點比較關鍵：Merge Request（簡稱：MR）、Discussion（簡稱：Diss）。

在這兩點基礎上，我們確定了幾個角色：

  • Owner（需求負責人，程式碼改動提交者，MR 發起者）
  • Reviewers（MR 參與者，前端團隊的同事，可能不止一個人。負責 Review 程式碼。）
  • Disser（某個 Reviewer。對某個 MR 發起 Discussion 的人。）

流程

  1. 對 GitLab 上需要進行 Code Review 的專案進行設定（Settings - General - Merge request settings - Only allow merge requests to be merged if all discussions are resolved）。
  2. Owner 在本地開發環境，某分支（以某功能分支 feat-example 為例）做好功能開發，充分自測後將程式碼推送到 GitLab。
  3. Owner 基於 feat-example 分支，發起目標分支為 develop 分支的 MR。MR 需要有儘可能詳細的描述。比如：需求文件地址，做了哪些修改，某個功能的設計實現思路，需要哪幾位 reviewer 對本次 MR 進行 Code Review 等。推薦使用 MR 模板。
  4. Owner 成功發起 MR 後，通過團隊協作工作告知 Reviewers 有 MR 需要進行 Code Review，以及 MR 的緊急程度。
  5. Reviewers 基於 MR 進行進行 code review。如果對 MR 有任何問題，在 GitLab 上針對具體程式碼進行 comment（發起 Discussion），review 完成後通知 Owner 結果（本次 MR 通過 / 本次 MR 有 n 個 Diss）。如果有 Diss，Owner 需要對每一個 Diss 進行回覆，直至所有 Diss 的狀態變更為 Resolved。
  6. Owner 對 MR 進行 merge 操作，並在測試環境釋出程式碼，通知相關 QA 同學測試，QA 測試通過後由 QA 通知產品和設計師進行驗收。（此處有一個細節：Owner 如果確定可以進行 merge 操作？我們想到有兩個方案：1. 以 Reviewers 通知 Owner 為準 2. 以 Reviewers 給 MR 點贊為準，因為 GitLab 上是可以對 MR 進行點贊操作的。目前團隊採用的是第2種方式。）
  7. 如果測試或者驗收環節發現問題，Owner 需要對程式碼進行修改，然後發起新一輪的 MR，直至測試環境程式碼通過驗收。
  8. 和 QA 同學確認程式碼可以釋出至生產環境，並進行程式碼釋出，通知 case 相關同學某功能已上線。

原則

在執行 Code Review 過程中，我們有一些原則需要遵守：

  • Owner 發起 MR 之前的程式碼需要進行充分自測
  • 程式碼版本控制 commit 的粒度不要太大
  • 不阻塞他人的工作，儘快響應他人的 Code Review 請求（這一點比較考驗團隊成員的合作精神、團隊意識。同時也要求開發者要合理安排自己的時間，要有能力隨時放下手中的工作，隨時繼續手中的工作）
  • 如果某個 MR 緊急，可以告知 Reviewers
  • 除有必要，否則 Owner 不要在提測驗收階段刪除分支（例如勾選“remove source branch when merge request is accepted.”），應等待分支合入master分支後移除，避免預發/測試分支重建時被遺漏。
  • 定期回顧和總結 Code Review 執行情況（比如在團隊週會時進行）

邊界

清楚了 Code Review 流程之後，其實還有一些邊界情況需要考慮。我會將團隊目前採用的處理策略寫出來供參考。

  1. 週末出現線上緊急 bug 要遵循 Code Review 流程嗎？可以不進行 Code Review，以快速修復 bug 為主。
  2. 某個需求（專案）留給開發時間非常緊張時怎麼辦？可以不進行 Code Review，優先保證按時需求（專案）上線。
  3. 團隊內部專案、組內同學個人發起的興趣專案是否需要進行 code review？決定權在專案 Owner。
  4. MR 遇到程式碼衝突怎麼辦？建議在 code review 之後，由 Owner 將程式碼拉取到本地進行 merge 並解決衝突，然後將最新程式碼推送到 GitLab（此時 GitLab 上 MR 會自動 merge 掉）。

收穫

坦言，在一個從未進行過 Code Review 的團隊想把這個機制運作起來，並不是一件容易的事情。尤其是在決定開始進行 Code Review 後的起步階段。但是如果能認準方向，團隊的成員齊心協力朝著既定的方向去走，最終會獲得如下的收穫的：

  • 團隊成員程式碼風格統一
  • 減小了專案交叉維護的阻力
  • 使新成員更快速融入團隊
  • 避免了低階 bug 在測試環境出現
  • 良好的技術交流氛圍

待完善

上面描述的這個機制並不是完美的。目前我可以想到的可以優化的點如下：

  • 優化編碼規範（技術本身在發展，團隊成員的水平在提高，隨之之前定下來的編碼規範也會適當的進行優化）
  • Check List（這一點實際上目前團隊已經開始做了。當業務具有一定複雜度後，某些業務邏輯的迭代難免會牽扯較多已有業務，此時如果有一份 Check List，會幫助 Owner 及 Reviewers 更好的進行 Code Review）
  • 激勵機制
  • 程式碼測試用例（主要是指業務程式碼增加測試用例。目前團隊也開始進行了一些嘗試。）
  • 自動化

寫在最後

將團隊在使用的 Code Review 機制以文字的形式沉澱下來，主要是想分享給更多的人。如果這些文字對某些人、某些團隊有幫助，那對於我來說是一件令人欣慰的事情。如果能接收到關於優化現有機制的指點，也會是一件令人開心和感激的事情。

此外，還想表達的一點是：丁香醫生前端團隊是一個非常在意每一個團隊成員成長的團隊。

我猜，你可能猜到接下來我要說什麼了。

是的，隨著丁香醫生業務的發展，我們需要優秀的前端同學加入我們，一起茁壯肆意成長。更多關於團隊的介紹，可以參考請問丁香醫生前端團隊怎麼樣？

招聘 JD

高階/資深前端工程師

職位描述

  • 負責丁香醫生旗下產品的前端開發工作（網站，Web App，Hybrid App，微信小程式，管理後臺，Node.js 中間層）；
  • 依據產品的需求，優質高效的完成前端專案的開發和維護；
  • 對產品的前端效能進行優化，確保產品具有優質的使用者體驗；
  • 參與丁香園前端團隊的基礎平臺建設；

任職條件

  • 3 年以上前端工作經驗；
  • 熟練使用 HTML（HTML5）、CSS（CSS3）和 JavaScript（ES6/ES7）；
  • 熟悉網路協議（HTTP/SSL）；
  • 熟練使用 Webpack 或者 rollupjs；
  • 至少熟練使用一種 CSS 前處理器（如：Less、Sass、Stylus）；
  • 至少熟練使用 Vue.js、React.js、AngularJS 三種框架中的一種；
  • 對前端開發規範、工程化、元件化、測試有一定的認識和實踐；
  • 理解並熟練使用物件導向程式設計思想，注重設計模式、模組化開發在實際專案中的應用；
  • 較強的責任心，良好的溝通能力和文件編寫能力；

優先條件

  • 在簡歷裡寫明 Github 賬號或個人部落格地址；
  • 獨立開發過或者參與過優質的開源專案；
  • 有實際 Hybrid App 專案開發經驗；
  • 有實際的微信小程式專案開發經驗；
  • 有高負載場景下 Node.js 應用開發和運維經驗；
  • 熟練使用 TypeScript；
  • 熟悉使用一門非前端的程式語言（如：Java、PHP、Python、Go）；

前端實習生（全職）

職位描述

  • 負責丁香醫生旗下產品的前端開發工作（網站，Web App，Hybrid App，微信小程式，管理後臺，Node.js 中間層）；
  • 依據產品的需求，優質高效的完成前端專案的開發和維護；
  • 對產品的前端效能進行優化，確保產品具有優質的使用者體驗；
  • 參與丁香園前端團隊的基礎平臺建設；

任職條件

  • 對程式設計技術有熱情，期望自己在技術上有快速成長；
  • 畢業前能夠全職實習至少 6 個月；
  • 熟練使用 HTML（HTML5）、CSS（CSS3）和 JavaScript（ES6/ES7）；
  • 熟悉網路協議（HTTP/SSL）；
  • 理解並熟練使用物件導向程式設計思想，注重設計模式、模組化開發在實際專案中的應用；
  • 較強的責任心，良好的溝通能力和文件編寫能力；

優先條件

  • 在簡歷裡寫明 Github 賬號或個人部落格地址；
  • 獨立開發過或者參與過優質的開源專案；
  • 熟練使用 Vue.js、React.js、AngularJS 三種框架中的一種；
  • 有實際 Hybrid App 專案開發經驗；
  • 有高負載場景下 Node.js 應用開發和運維經驗；
  • 熟練使用 TypeScript；
  • 熟練使用一種 CSS 前處理器（如：Less、Sass、Stylus）；
  • 熟悉使用一門非前端的程式語言（如：Java、PHP、Python、Go）；

既然已經看到這裡了，不如發一封郵件我們聊一下吧：lizy@dxy.cn。

本文作者：丁香園前端工程師 @志遙

相關文章