[譯] Pull request review 的十大錯誤

• 原文地址：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、前端、後端、產品、設計 等領域，想要查看更多優質譯文請持續關注 掘金翻譯計劃。