我參加過很多 GitHub 託管的專案，不管是個人的、開源的還是合約的。有時候用免費的 GitHub，其他時候用 GitHub 企業版。但有一件事是一樣的：提交合並請求（Pull Request）很容易的，但審查好 PR 真的很難。

所以乾脆寫一篇合併請求的 10 個常見問題以及對於如何做得更好的建議！

1. 不佳思索的贊同

事情就是這麼有誘惑力。這次的合併請求規模真的很大，提交者又是你信任的某個人。他們已經在這份程式碼上工作一段時間了，而且一直都處理的很好。不要說你還有自己的截止日期（deadline）要跟。

振作起來吧！

你需要真正花時間去審查那份程式碼。每個人都會犯錯——再有資歷的人也沒有魔法棒。而你作為一個審查者，你的工作就是用你的創造力和專業知識減少這份合併請求以任何方式讓程式碼庫變得更糟糕的概率。

這才是真正的目標，為什麼不是呢？如果每個合併請求都能讓程式碼庫更好，那這個專案就確實有長期發展的潛質。

2. 拖延

畢竟是一個這麼大的合併請求，為什麼非要現在稽核？而且你現在的任務實在太重要了。你最終會抽時間來處理它的，是吧？或者你可能只是在等其他人發表意見……

捫心自問，讓原力貫通全身！在你內心抵制的背後可能會有一些真實的抱怨。

既然你已經明確了你的關注點，那就開始行動吧！

• 如果提交者沒有提供足夠的引導告訴你這些修改都做了哪些事，那就去跟他要！例如，最初的需求文件在哪裡？
• 如果這些修改內容太多了一次審查不完，那就讓他們分批提交！
• 如果出現你不理解的東西，那就要讓他們給你講清楚！
• 如果你發現一大堆問題/關注點，那可能就是需要和提交者面對面溝通的時候了。

3. 標準 diff

你是否正在審查一些亂七八糟的程式碼？GitHub 和企業版 GitHub 的預設 diff 工具是“標準 diff”。這種模式下，為了把一系列的修改都表示在一個檔案中，GitHub會關注增加和刪除的行並很智慧地將修改編成組塊，而且這些都是行內表示。但是你知道嗎，大多數情況下，“標準”diff是很難閱讀的。所謂智慧的組塊選擇其實一點也不智慧。

好在 GitHub 和企業版 GitHub 現在都支援“分片”diff。左邊是舊檔案，右邊是新檔案。如果刪除程式碼那右邊就是空白，如果增加程式碼那左邊就是空白。不論哪種，你都可以清晰地看明白這檔案之前和之後是什麼樣子，有助於做出更好的審查決策。

不要去解決混亂。點選 diff 頁面右上角的“Split/分片”。

4. 風格蓋過本質

審查合併請求的時候除非必要請花盡量少的時間去討論程式碼風格和格式。我之前寫過關於使用 ESLint 這類工具把這些事情完全自動化的必要性。為什麼呢？因為浪費時間！

合格的程式碼審查者應當把時間花在理解這些程式碼修改的根本目的，就是去回看最初的需求。這對應哪一條工作項？這是一個特定處理？它到底想要完成什麼？

只有瞭解上下文環境才能做到真正的程式碼審查。有些在審查表面結構／風格的時候，看起來合理的地方在你理解了終極目標之後可能就變得不可接受了。

好吧，現在你可能想要回避提出這麼大一個問題，因為已經在現在的程式碼更改上花費太多時間了，但討論出一個更好的解決方案是很值得的。這對每個人都是一個學習機會。也可能你覺得會有更好的解決方案這個想法就是錯誤的，但你需要和最初的提交者討論之後才能發現這個事實。

5. 沒有捕獲不完善的修改

diff 工具確實能很清晰地告訴你哪些地方有修改。但這也是問題所在！顯然它們沒告訴你哪些地方沒有改變。要當心有些本應該應用更廣泛的修改，像 find/replace 這類的可能沒有覆蓋整個程式碼庫。

或者說是一個修改只完成了它本應修改的元件的子集。

或者根本就沒有測試。測試對於任何修改都是很重要的部分，但如果測試不出現在diff工具裡，確實很容易忘記。而且也不會有人提示你想起來測試。

不得不承認，這真的很難！這是程式碼審查中最難的。在提交者的分支或是你自己電腦的分支上做一些快速健康檢查可能會有些幫助。或者你可以諮詢提交者他們除了你能看到的程式碼之外還做了哪些綜合詳盡的測試檢查。

6. 粉飾太平的測試程式碼

合併請求裡只要有一些測試程式碼，很容易就讓人產生這很安全的錯覺。如果他們把測試考慮進去，那他們一定是高質量且綜合全面的。是這樣的嗎？

錯！

測試是一門藝術。恰當地平衡風險規避和測試代價以及是否適合程式碼覆蓋範圍和團隊文化。審查合併請求是團隊建立共享背景的絕佳時機。

還有一些問題需要考慮：

• 測試標題能否充分描述目的？
• 一些關鍵場景是否能捕獲？
• 是否有足夠的邊界測試樣例？
• 一個測試會執行應用程式的哪些模組？太多？或者太少？
• 測試是用恰當的斷言寫的嗎？是否失敗過？會經常失敗嗎？
• 如果測試失敗，能否快速追蹤錯誤？
• 如果有新的前端行為新增，會被加進手工測試指令碼嗎？看看自動測試？

7. 低估前端的複雜度

如果 CSS 和 HTML 有修改，會傾向於把它看作和演算法程式碼更改一樣。看到格式良好的修改就會想象他們在瀏覽器裡如何執行。“看上去很合理，”你說。

但是事情不會這麼簡單的。終端使用者看到的結果來自你的應用和各種渲染引擎的複雜互動。

跳出固有思維，把分支拉取下來。在各種瀏覽器和不同的屏寬比裡嘗試。因為這些事情真的很容易迷惑人。即使是資深的前端開發人員，也不要完全信任自己一直盯著的事情。這也是 CodePen 和 the like 存在的原因！

8. 思維狹隘

還有一個地方可能會因為 diff 裡格式良好的程式碼讓你昏昏欲睡。但考慮大規模情況是很重要的。目前專案裡的新程式碼，到底有什麼變化？會發生什麼？

可以從這些問題開始思考：

• 這會影響使用者的下載量嗎？會感知到效能變化嗎？對使用者體驗的改變是否足以需要在版本註釋裡說明，或是給使用者一封郵件說明？
• 它是否會引入新的程式碼或特性？是否需要新的測試方法、新的日誌或監控技巧，或者會改變部署過程？
• 使用者今天能否執行，或者需要等到一個標記訊號之後？什麼系統能夠原地驗證標記之後的事情？
• 綜合完善的測試需要多大難度？預發環境和線上環境到底有什麼區別？

9. 短期思考

有些合併請求裡有相當多的往返，可能是因為意見不一致或者是闡明的需要。這確實是一件好事情—它在建立共享上下文環境。但是當下一個開發人員來了並拿到這份程式碼會發生什麼呢？他們可能就沒那麼容易理解這場討論了。

未來建立可理解的上下文環境的一些建議：

• 在程式碼註釋裡充分體現關鍵的合併請求爭論。
• 修改審查者很難理解的程式碼—其他人未來也一樣理解不了！
• 在專案裡建立一個存放概念文件的地方，這個文件覆蓋更多相關的、廣泛應用的話題。
• 確保程式碼裡所有的TODO和工作項集合裡的每一項都是對應的，要足夠詳細到讓除了最初報告人之外的人也能執行。
• 程式碼審查的時候，考慮程式碼的長期維護—是否以後也能易於修改？線上環境是否易於維護呢？長期維護的代價是什麼？

10. 審查修正的時候草草了事

終於，你集中注意力完成了這個合併請求，並將它返回給提交者。也給出了反饋，提交者回應正在修正。

現在不是忘掉這個合併請求的時候。你確實是已經和提交者討論完需要的修改了，但不意味著這些改變都會是正確的！或者說全部都能修正！

修正合併請求是開發人員所做的風險最高的修改，因為每個人都想趕緊繼續下一件事。開發過程的關注越少，審查過程給的關注也就越少。

好吧，即使你已經非常詳細的審查過這個合併請求了，但還是要尤其注意最初合併請求裡的任何更新。如果新的修改被劃分進一個單獨的提交，那還好辦。如果整個合併請求都被重新復位/壓縮，那就讓人頭疼了。

這不是一件簡單的事情！

設計並實現軟體是一件很難的工作。那為什麼審查會更容易呢？事實上，審查可能會更難，因為你要用少得多的時間去建立一個正確的上下文環境以提供合理的反饋。

但我們也不能放棄審查，因為它實在太重要了！可以把本文當作一個起步的檢查表，或者以此為靈感建立一個新的檢查表。久而久之，你和你的團隊就可以給那些很重要但容易被忽視的考慮點建立一個定製的備忘錄。最終，你們的合併請求過程會形成一個強大的反饋迴圈，能夠提升團隊文化和程式碼質量。