- 原文地址：Top ten pull request review mistakes
- 原文作者：Scott Nonnenberg
- 譯文出自：掘金翻譯計劃
- 譯者： luoyaqifei
- 校對者：xiaoyusilen，phxnirvana
Pull request review 的十大錯誤
我已在多個 GitHub 託管的專案中工作過，無論是 個人的、 開源的 或者 工作中的。有的時候使用公開的 GitHub，別的時候使用 GitHub 企業版。但是有一件事情是一樣的：提交一個 pull request 實在是簡單，但是很好地 review 一個 pull request 實在是有點難。
為了避免更多的麻煩，本文會涉及到十大 pull request review 錯誤和一些怎樣做得更好的想法：
1. 漫不經心 +1
這是那麼地讓人難以抗拒：某個 pull request 實在太大，提交者又是你很信任的人。他們已經在這段程式碼上工作了很久，而且這段程式碼總是工作得很好。更不必說，你也有你自己的 deadline 要趕啊！
+1
看起來挺好
走起，合併吧！複製程式碼不要再這麼做啦！
你真的需要花一些時間來 review 這段程式碼。每個人都會犯錯——資歷水平並不是什麼對抗錯誤的神奇守衛。並且，你要清楚自己的身份，作為一個 reviewer，你的身份是使用你的創造性和專業性，使用任何方式來減少 pull request 將程式碼庫變得更糟的情況。
這才是真正的目的，對不對？如果每個 pull request 都讓程式碼庫變得更好，專案很可能是有長期潛力在的！
2. 拖延
為什麼現在就要 review 它呢？畢竟這真的是個大 pull request 啊。你當下的任務太重要了。你最終會繞著它走，對不對？或者，可能你只是等著別人插手吧……
拷問下你自己的內心吧！讓那股力量從你的心中流過！在那種阻力下，你可能會有一些真正的顧慮。
既然你已經認清了你真正的顧慮，行動起來！
- 如果對於更改到底做了什麼，程式碼提交者沒有提供足夠的指引，直接去問提交者要這些！比如說，原始需求在哪？
- 如果只是因為更改太龐大，難以一次 review 完畢，讓他們分成多次提交！
- 如果你不明白什麼，放下你的驕傲，問！
- 如果你發現了足夠多的問題／有足夠多的顧慮，可能是時候與提交者做一些面對面的互動了。
3. 統一 diff
你在 review 不知所云的程式碼嗎？在 GitHub 和 GitHub 企業版上預設的 diff 顯示是「統一的」（Unified）(譯者注：現在 GitHub 已經預設使用 Split 顯示了)。在這種模式下，渲染一系列檔案的更改，軟體看的是被新增的／被刪除的行，並且嘗試著將更改塊智慧分組，所有的都是內聯的。但是你能看懂多少更改呢？在多數情況下，「統一的」 diff 很難閱讀。所謂智慧的塊選擇真的不夠智慧。
好訊息是 GitHub 和 GitHub 企業版都支援將 diff 「分屏」（Split）。左側是舊檔案，右側是新檔案。如果程式碼被移除，你將在右側看到空區域；如果程式碼被新增，你將在左側看到空區域。這兩者都可以讓你清楚地看到檔案在更改前後的模樣，能夠促使你做更好的 review 決策。
不要為莫名其妙買單。在 diff 的右上角點選「將 diff 分屏（Split）」吧！
4. 專注樣式而不是實質
在 pull request 的 review 時，即便對程式碼風格和格式有異議，也不應在這些方面浪費時間。我之前寫過一篇文章，關於 使用 ESLint 這類工具來將這些事情完全自動化的必要性。為什麼？因為它完全是浪費時間！
一個好的 reviewer 會將時間放在嘗試著去理解 程式碼更改的最終目的 上，通過回溯到原始的需求。有一個追溯這段更改的工作項嗎？有相應的測試用例嗎？它到底在要什麼？
只有掌握了這些程式碼背景，真正的 review 才會實現。可能在浮於表面的結構／樣式 review 時看起來合理的程式碼，當理解了終極目標後會變得不能接受。
當然，你可能會迴避惹上這種「大」事情，畢竟有如此多的時間被消耗在已有的更改上了。但是，討論更好的解決方案是值得的。對於每個人來說，這都是一個學習的機會。你可能錯誤地相信會有一個更好的解決方案，但是這種相信會引領你和原始提交者進行一次討論，來確定到底有沒有更好的解決方案。
5. 不注意未完成的更改
差異對於展示已有的更改很棒，但是這也是問題所在。從定義上就可以看出來，它並不能展示出什麼 沒有 被更改。時刻觀察有哪些更改應該被更廣泛地應用，比如說查詢／替換這種可能沒有覆蓋到整個程式碼庫的情況。
或者一個更改應該涉及到一整個元件而它只涉及到其中一部分的情況。
或者完全缺少測試的情況。測試是更改很重要的一部分，但是它實際上是很容易被遺忘的，如果它們根本不在 diff 裡面的話。你很難因為被提醒而想起它們。
不得不承認，這真的很難！這是 review 裡最難的型別。它可能幫助你做一些快速的明智的檢查搜尋，或者在提交者的分支上，或者在你自己的機器上有的任何的東西。或者，你可以問提交者他們在你能看到的程式碼更改之外，做過哪些型別的全面檢查，
6. 輕視測試程式碼
一旦在 pull request 裡有一些測試程式碼的更新，就很容易被麻痺在一種錯誤的安全感裡。如果他們將測試程式碼放入了，這些測試程式碼一定是高質量的、易理解的，對嗎？
錯！
測試是一門藝術。它使用了很多的上下文來恰當地平衡風險轉移和測試消耗，以適合程式碼領域和團隊文化的方式。Pull request review 是一個很好的、團隊可以建立這種共享上下文的地方。
有一些問題需要考慮：
- 測試標題合適地宣告瞭嗎？
- 關鍵場景被捕獲了嗎？
- 為了安全起見，足夠的邊緣用例被覆蓋了嗎？
- 哪部分應用被單個測試使用了？太多？太少？
- 測試斷言寫得好嗎？測試掛過嗎？測試經常掛嗎？
- 如果一個測試掛掉了，容易追溯到錯誤原因嗎？
- 如果加入新的前端行為，它有被加入 手動測試指令碼 裡嗎？有被加入 瀏覽器自動測試 裡嗎？
7. 不考慮前端複雜性
如果改動發生在 CSS 和 HTML 裡，人們的傾向是將它當作演算法程式碼改動來對待。你會看到看起來很規範的改動，並且想象它們會在瀏覽器裡做些什麼。“看起來很合理”，你說。
但是事情不是這麼簡單的。使用者最終看到的效果來自於你的應用和不同的渲染引擎之間的複雜互動。
不要只腦補了，把分支 pull 下來。在多種瀏覽器和螢幕尺寸上試試，因為這種頁面改動是很微妙的。就算你是一個專家級前端開發人員，也不要相信你自己能夠靠肉眼搞定這種改動。這也是 CodePen 和 the like 存在的意義！
8. 狹窄眼界
這是另一個你可能會被 diff 裡看起來很規範的程式碼所麻痺的領域。從大的角度來考慮問題是很重要的。有了專案裡的這段新程式碼，會有什麼改變？可能會發生什麼？
有一些你可以著手的問題：
- 這個改動會影響使用者的下載量嗎？對於效能的影響有多大？會改變使用者體驗，以至於需要放入版本的釋出說明，或者放入給使用者的郵件裡嗎？
- 它會引入一種新型別的程式碼或者特性嗎？它需要新的測試方法，新的日誌方法、監控技術，或者需要部署流程的改變嗎？
- 它會被使用者今天用到嗎，或者它在一個 flag 後面？存在什麼系統可以檢測 flag 後面的東西呢？
- 測試完全有多難？在開發環境和生產環境裡可能會有什麼區別呢？
9. 短視思維
在一些 pull request 裡，有一些總在反覆的東西，可能是因為不同意，或者只是需要闡明。這種做法很棒——這是在建立共享上下文。但是當下一個開發者遇到這段程式碼的時候，會發生什麼呢？他們會對這段討論難以理解。
為未來建立可以理解的上下文，有這麼些想法：
- 在程式碼的註釋裡放入關鍵的 pull request 討論。
- 改掉對於 reviewer 來說難以理解的程式碼——這些程式碼在未來對於其他人來說同樣會難以理解！
- 在專案裡建立一個存放完整的概念文件的地方，包括一些涉及到的、廣泛應用的主題。
- 確保所有程式碼裡的
TODO與你的工作項資料庫中的某一項相對，並且有足夠的細節能讓它被原始報告人以外的人操作。 - 當 review 的時候，請考慮對程式碼的長期維護——改變會簡單嗎？在生產環境中維護簡單嗎？長期消耗是什麼？
10. 對修正進行 review
終於！這個 pull request 看起來引起了一些注意，並且又回到了提交者這邊。你已經給出了你的反饋，提交者正在相應地作出更改。
現在不是忘掉這個 pull request 的時候。你已經跟提交者討論好了有哪些需要更改的地方，但是這不意味著這些更改將會是對的！或者甚至完全是錯的！
對 pull request 的修正是開發者有史以來可能做出的風險最大的更改，因為所有人都只想前進。如果一個人在開發中給予的關注不夠，那麼對 review 也不會太關注。
尤其要密切注意對最初的 pull request 的任何更改，即使你已經完整地 review 過 pull request。如果新的更改被放到了單獨的提交裡，那麼情況要簡單些。如果整個 pull request 被重新 rebased/squashed，那麼這就讓人有點沮喪了。
這不容易！
設計與實現軟體是一件難事。憑什麼 review 它就會簡單一點呢？實際上，review 很大可能更難，因為比起寫程式碼來說，review 能夠用來建立正確的程式碼背景，從而提供合理的反饋的時間更少。
但是我們不能放棄——它很重要！
將本文作為你 review 的入口清單吧，或者讓它在這方面激勵你。隨著時間的推移，你和你的團隊將會建立一個自己獨有的清單，用於提醒 review 程式碼時一些重要但是容易忘記的點。最終，你的 pull request 流程將會變成一個強有力的 反饋環，能夠提升你們團隊的 review 文化和程式碼質量。
掘金翻譯計劃 是一個翻譯優質網際網路技術文章的社群，文章來源為 掘金 上的英文分享文章。內容覆蓋 Android、iOS、React、前端、後端、產品、設計 等領域，想要檢視更多優質譯文請持續關注 掘金翻譯計劃。