在 GitHub 上玩轉開源項目的 Code Review
一、幕後故事
時光荏苒，歲月如梭…… （🤮太文縐縐了，這不是我的風格）
今天我準備聊聊在 GitHub 上如何玩 Code Review。
突發奇想？心血來潮？不是。
咋回事呢？（對八卦不感興趣的可以直接跳到下一節，但是我猜你會感興趣。）
首先我是 DevStream 開源社區成員。
在五月份，又有3位活躍的優秀的牛X的 Contributors 正式加入 DevStream 開源社區，正式成為了社區 Member！
(看下面的紅框框)
於是乎，加上三月份的4個「老 Member」，DevStream 社區就有7個「社區 Member」了（社區 Member 區分於像我這種在思碼逸上班的內部 Member）！
7個瘋狂輸出的 Members，外加接近20個 Contributors，我和鐵心兩個人基本就只能看著 pr 笑笑，一邊表示欣喜，一邊表示 review 不過來了，「應接不暇」，廢了廢了……
也就是說，是時候組織一個 reviewer 組，拉著大家一起玩 Code Review 流程了！
說到 Code Review 流程，流程是啥？規範是啥？規則是啥？技巧是啥？xxxx？我能預想到 reviewers team 這個事情落地之日會有一堆問題砸到我頭上。好吧，我需要寫一篇文章來聊聊這些事。
二、踏上旅途
下面我們開始一次 Code Review 之旅。
1. 搶票階段：認領一個 Review 任務
開始一次 review 之前，首先咱得「認領」一個 review 任務。
怎樣算成功認領？如下圖，Reviewers 里有你的頭像，這時當前 pr 你就是 reviewers 之一，同時可以看到黃色 bar 里的一行字「This pull request is waiting on your review.」以及綠色的按鈕「Add your review」。你可以點擊這個「Add your review」開始一次 Code Review 之旅。
「那麼怎麼認領呢？」 可能你還想問我。
這個問題有答案，也沒有答案。
因為你是 reviewer 之一，那麼你就有許可權自己點擊 Reviewers 右邊的⚙️齒輪按鈕，然後指定自己是一個 Reviewer。如果你不是一個「合法」的 reviewer，那麼你得先成為 reviewer (If you want)。
2. 持票上船：開始 Review 流程
點擊 Add your review 按鈕後，咱就進入到了網頁版 Code Review 頁面，大致如下：
這裡有很多值得「探索」的特性，比如：左邊的「文件樹」、文件樹上方過濾「commits」的下拉框、右邊的「文件過濾」、每個文件右上角的 Viewed 選擇框、…… 每發現一個新功能，你的 review 過程就會多簡化一分。
關於這個頁面，沒有比官方文檔更權威和詳細的介紹了，我想我沒有理由「搬一次磚」，大家點擊鏈接進一步探索吧。
對於簡單的修改，或許網頁直接查看程式碼 diff 就足夠了，類似這種變更級別的 pr：
我們可以輕鬆判斷這個修改是不是「正確」，然後選擇進一步的操作，比如 Approve：
3. 下船休整：下載一個 pr 到本地 Review
對於一個不能「一眼看穿」的 pr，比如對方沒有附上詳細的測試結果等資訊來證明他的修改已經「充分測試」（充分測試的例子），這時候靠肉眼我們很難判斷這個 pr 合入後會怎樣，或者不藉助 IDE 的能力我們甚至很難看懂一些複雜的修改，這時候就需要下載這個 pr 對應的程式碼到本地，然後用 IDE 來幫助 review 了。
以 pr #641為例，我們需要下載這個 pr，這時只需要執行這樣兩條命令：
git fetch upstream pull/641/head:pr641
git checkout pr641
效果如下：
然後在 IDE 里打開項目，就能看到 pr 對應的程式碼了：
「程式碼在手，天下我有！」
這會你可以在本地仔細查看每一處程式碼細節，可以在本地跑一下 make buid -j8 或 go test ./... 之類的命令來逐步「打消自己內心的疑慮」。當然，pr 本身會觸發 GitHub actions workflows，這些基礎的 build/ut 流程其實本地不跑也能知道是不是有錯誤，我們可以直接在頁面上看到（每個項目具體的 ci 邏輯不一樣）：
到這裡，你就基本能夠完全看懂一個 pr 對應的所有修改了，是放他過？還是攔下馬？他的「命運」掌握在你的手裡！
三、移花接木：提交一個 commit 到別人的 pr
可能有時候，你需要修改別人的 pr。哥們，我建議你抽根煙冷靜一下，再問自己一句：我真的必須去修改他的 pr 嗎？這樣做會不會被打？
一般情況下，我不建議你去修改別人的 pr，尤其是不能保證你的修改一定正確的情況下。因為你別人的 pr 本身就是容易冒犯到別人的事情，其次萬一你改了之後發現還需要別人自己補一個 commit，他會在複雜的 git 操作中罵你一萬遍。
什麼時候需要去動別人的 pr 呢？舉個例子，比如這個 pr。
首先這是一個 new contributor 提交的 first pr，所以我不希望他的 first pr 之旅太艱辛。然後這個 feature 其實並不簡單，雖然技術上看並不難，但是要做到「不重不漏」就需要對 dtm 命令的所有子命令都「瞭然於胸」才行。顯然，這對一個 new contributor 來說要求太高了。所以在他提交了一個 commit 之後，我直接在這個 commit 的基礎上又加了一個 commit，完成了剩下的工作，並且在認可他的工作後告知其為什麼我要修改這個 pr，並附上了測試結果等。
具體怎麼操作呢？步驟如下：
- 修改程式碼，本地 commit
前面我們已經下載了一個別人的 pr 到本地，接著自然是繼續修改，然後提交 commit（git add xxx & git commit -s -m "xxx"），到這裡都沒啥技術含量，不贅述了。
- 找到 pr 對應的源分支
在 pr 頁面可以看到具體 pr 是從哪裡提交的，比如：
我們點進去，就可以找到 fork 項目的地址資訊，像這樣：
於是，我們可以這樣來加一個 remote：
git remote add himku [email protected]:himku/devstream.git
此時這個 pr 對應的 fork 項目的地址是 himku([email protected]:himku/devstream.git)，對應分支是 fix-issue-559，於是我們可以這樣將自己新增的 commit(s) 提交到這個 pr 里（本地 commit 後執行）：
git push himku HEAD:fix-issue-559
是不是很酷？
三、亂七八糟的規則：目的是啥？規範是啥？要求是啥？啥是啥？
再聊聊剩下的一些零零碎碎的問題，比如可能你想問 review 的目的啊，規範啊，要求啊，啊啊啊……
1. 目的是啥？
可能你會問我為什麼要 code review ？我希望你別問。因為我不想總結。（這個問題可以 Google 到一堆答案）
我相信你心中有答案，code review 對應的是一堆的「褒義詞」，比如：發現錯誤、保證品質、互相學習……
你想的都是對的，總之這個事情值得做就對了，不需要去總結為什麼。
啥？
你還是想聽我談談？
談談就談談。
- 軟體品質：首先大多數的錯誤可以在 code review 階段被暴露出來，這些錯誤留到日後「爆雷」再被修復，代價會大很多，不管是造成的業務負面影響，還是額外付出的修復時間。所以 code review 看似多花了時間，其實整體看是提升交付效率的；
- 程式碼品質：嚴格執行 code review 流程一方面可以互相督促對方：你別隨便提交「垃圾」程式碼上來，會有人 review；一方面假如有「垃圾」程式碼被提交上來了，可以有人站出來及時制止。完善的 code review 流程可以避免程式碼可讀性日益變差，維護成本日益增大，逐步變成「屎山」；
- 傳播經驗：如果我寫的程式碼很漂亮，我希望你來 review，這時候一方面我想「秀」，無需避諱；另外一方面我希望你能夠學著點，這是為你好；如果我的程式碼寫的很爛，我希望你來 review，這時候我希望你可以給我指出問題，幫助我提升編碼水平，這是為我好；總之互相 review，互相學習，你好我好大家好；
2. 規範與要求是啥？
當我們解決了所有「流程」或「技巧」層面的問題後，下一個「非技術性」問題是：怎樣的程式碼需要「返工」？怎樣的程式碼可以被合入？
我相信你心中會有這樣的疑問。
對於有「錯誤」的程式碼，毫無疑問，不能合入，這不在我們的討論範圍。
那麼正常運行，沒有邏輯錯誤的程式碼呢？比如「程式碼風格有點混亂」，比如「缺乏必要的一些注釋」，比如「可讀性差」……
我們分三個層次來思考：
- 嚴格：我們完全可以指出任何是「問題」的問題，因為一份 WTF 的程式碼被合入後，所有對 coder 的罵聲都包含一句潛台詞「reviewer 幹啥吃的？」所以很簡單，你覺得有問題的地方都可以提出來，包括：
- 「程式碼太複雜，看起來費力，我覺得可讀性不好。」
- 「我看了十分鐘還是看不懂，說明可讀性不夠好。」
- 「這個函數太長了，我滑鼠滾了好幾下才看完，你應該拆分一下。」
- 「這個函數從名字我看不出來功能，但是我懶得看具體邏輯，為什麼沒有更多的注釋？」
- 「你這個源文件都五百行了，你要不拆分一下？」
- 「這個包的邏輯很關鍵，你應該加點 UT？」
- ……
-
一般：如果一份程式碼功能完全正確，可讀性也勉強還行，或許沒有很好的面向對象來組織，或者注釋不太詳細，或者存在其他一些小小的不完美。這時候你也可以選擇通過，避免太嚴格的 review 規則把一個 pr 的合入周期拖的太長，一方面影響交付效率，一方面打擊開發者信心。很多時候我們可以對自己，或者對核心開發者要求更嚴格一些；但是對於社區貢獻者，往往需要更多的「寬容」與認可。
-
溫柔：如果是一個 new contributor 提交的一個 first pr，這時你可以放下各種能放下的要求，只要這份程式碼還過得去，就能合入，沒有啥比給新人一些信心更重要的。如果 pr 存在一些小問題，你覺得對他來說改起來不會太困難，你可以留言友好的告訴他哪裡需要改，怎麼改；如果你覺得對他來說進一步做到「完美」有難度，你也可以直接提一個 fix 的 commit 到這個 pr 里，直接幫他修復問題，然後留個言告知他沒有考慮到的問題是什麼。
終點站到了，下船！
今天就聊到這裡。
意猶未盡？
再去看看 Google 的 Code Review Developer Guide 吧！
- 你可以收藏我的個人網站 //www.danielhu.cn，閱讀更多我寫的部落格文章；
- 你也可以關注我的個人微信公眾號「胡說雲原生」，第一時間接收新文章推送；
- 當然，你也可以收藏 DevStream 的部落格網站 //blog.devstream.io 或者官網 //www.devstream.io，裡面不止有我這種「不嚴肅」的人發的「主要用來搞笑」的文章，還有 DevStream 團隊其他成員發的「嚴肅講知識」的「科普文」。
