前言
本文原作者 Mark Seemann, 10-tips-for-better-pull-requests, 15 January 2015, 譯者 Benjamin Rice (HENG CHIA, FAN)
1. 保持 PR 輕量
一個夠小、專注一件事情上的 Pull Request (以下用 PR 稱呼)對於 PR 的接受度會有很好的幫助。
當我收到 PR 通知的第一件事就是去看這個 PR 的大小。嚴格的審核一個 PR 是很花時間的，在過往的經驗裡審核 PR 花費的時間會依據大小呈指數成長而且裡面的關係基本上會很複雜。
若是在一個開源專案(Open Source project) 中收到 PR，我會意識到提交者很可能在他的空閒時間內投入了大量的工作，所以我也會投入一定的時間去審這個 PR ，即使我認為這個 PR 的 sizs 太大，尤其是當這個 PR 看起來像是首次貢獻者送出的。不過，若 PR 很大的話依舊需要排出時間來審，我無法用片段的時間來審一大段程式碼。
但若是在一個職場環境下收到 PR（例. PR 發起人是一個受薪的程式人員），我通常會因為 size 的關係將 PR reject。
至於 PR 要怎樣才夠小呢？顯然這個問題跟 PR 要處理的需求有關，不過一個 PR 少於一打的檔案還不錯。
譯者個人想法： 還滿認同這邊的論點，首先一個 PR 若東西太多會需要花費更多的時間去看，再來通常 PR 若是包了太多東西，若不是原需求太過複雜就是裡面偷渡太多跟需求無直接相關的內容。不過這件事情通常跟如何拆解一個需求有很高的相關性。而需求的拆分能力牽扯的範圍除了需求的理解、解決方案的選擇、既有框架的限制以及專案的上線時程與範圍等等，實在有點廣泛就不再這邊細聊了。不過這邊也是有一些例外的情境，像是專案使用的語言升級產生的異動(e.g. python2 upgrade to python3)，或是有些靜態檔案從專案移除搬去其他地方，諸如這種情況都會產生很多檔案的異動，我覺得反而這些放在一起比較好管理。
2. 一個 PR 只專注一件事
就像單一職責原則(SRP) 所述，一個類別(class) 應該只能有一個職責，一個 PR 應該也只針對一個事情。
想像一個反例，你送出一個 PR 裡面包含了三個獨立、不相干的影響（我們先用 A, B 和 C 來稱呼）。Reviewer 可能會認同 A 跟 C 這兩個部份，而且你提出的解決方法也是合理的。但是 Reviewer 對於 B 卻還有些問題。也許 Reviewer 認為 B 並非什麼要修的問題，或是對於你提出的解決方案並不買單。
這會是一段冗長討論問題 B 的開端，以及如何解決這個問題。這個討論可能會耗時數日（特別是討論者所屬時區不同），才會有機會得到一個一致的結論。也許你需要調整你的 PR 來滿足 Reviewer 的要求，這些事情都很花時間。
事實上，這花上的時間很可能可以讓很多 commit 都審完並 merged into master branch (or main branch)，而你的 PR 在準備 merge 時因為落後太多以至於發生衝突(conflict) 不能自動完成 merge。歡迎來到合併地獄(Merge Hall)。
譯者個人想法：這個算是第一個要點的延伸，當 PR 只專注在一件事情時 PR 就容易比較小。就如同我認同第一個要點，這個要點也是相當實用，同時也可以幫助自己在處理 commit 時整理自己的想法。不過在實務上，我們有個時候會在處理需求時在異動檔案上順便做 formatting 的改動，這個部份我是覺得可以用獨立的 commit 包在 PR 裡就好，前提是這些異動僅限此次 PR 有改到的檔案。
3. 注意每一行的字數
Reviewer 大多數的情況下會用 diff tool 來審 PR。GitHub 與 GitLab 這類提供以瀏覽器為基礎的工具。Reviewer 甚至可以設定工具讓他可以並排著比對差異；這樣可以讓異動的差異更容易讀懂，但同時也意味著程式碼(code) 本身的易讀性在使用一半螢幕的情況下也要能滿足。
若你一行太長的話，就會強迫 Reviewer 水平拉動視窗來閱讀。
有很多建議一行保持少於 80 個字元原因；讓你的 code 更容易被 Reviewer 審閱是個可以被加入這個清單中的理由。
譯者個人想法：這個我基本上是覺得可以遵守，但是要小心不要因為縮短長度犧牲 code 的可讀性 (code readability)，像是使用不常見的縮寫作為變數名稱，若是這樣的話就本末倒置了。另外字數長度比較相關的就是 commit 的標題描述其實也可以盡可能簡短，詳情可以參考這篇，裡面寫的相當好，值得參考。
4. 避免重複 format 文件的 PR
你或許會有強烈的慾望想要改變既有 code 的樣式(format) 以符合你的風格。拜託，先萬不要這樣做。
每個你更動的內容都會顯示在差異裡。一些差異檢視工具可以忽略空白格的差異，但即使可以忽略這些差異，這些工具能忽略的差異依然相當有限。特別是，工具不能忽略你把 code 搬移到別處去，所以請不要 re-format。
若你真的很想解決空白格的問題、把 code 移到別處去、更換 format 或是針對 code 改變風格。請在另外一個 PR 來做，並且也只做這件事情，並在你的 commit 裡也註明這個異動。
譯者個人想法：Hummm…這個作法會讓 PR 以及 commit 更漂亮，整理的更乾淨這不能否認，不過這樣的習慣是需要被養成的，當團隊的成員尚未養成這樣的習慣前就是個陣痛期，所以我個人可以接受偶而在 PR 裡偷渡一些修改樣式的 commit，當然這些異動最好就集中在該 commit 裡，這樣一來可以有基本的整理效果，二來大家也都輕鬆一點。
5. 確保 code 能成功 compile
在送出 PR 之前，在本機建置(build) 看看。確實，「It works on my machine」 並非特別有用，但這可是最低門檻。若連自己的電腦都 build 不過了，那顯然在別的人環境裡也不會有理由可以 build 過。
注意看編譯器(compiler) 拋出的警告訊息。這訊息可能不會阻止你編譯(compile)，所以你可能不會特別注意這些訊息。然而，若你的 PR 造成更多 compiler warnings，Reviewer 可能會拒絕這個 PR，像我就會。
若這個專案有建置腳本(build script)，就嘗試用該 script 來 build，並且只在 build 成功後送出 PR。在我眾多 open source 專案中，我的 build script 會將警告訊息以錯誤訊息的形式拋出。這一份 build script 可能套用預設或是實踐了多種規範給個別的 code base。在你送出 PR 前執行它吧，因為 reviewer 很可能會在 merge PR 前先執行過 build script。
譯者個人想法：恩，說得很對。應該說每次下 commit 時都該如此。這個要求很像是 clean code 裡面提倡的童子軍原則(The boy scout rule)，你要讓 code base 在你經手過後變得更好，不能放任 code 散發出壞味道(bad small) 而不處置。
6. 確保所有的測試都是通過的
假定我們正在處理的 code base 都有自動化測試，那就請確保在發送 PR 前，所有的測試都能通過。
這點應該不需要再多說什麼，但我定期還是會收到執行測試會失敗的 PR。
譯者個人想法：完全認同，送出 PR 前應該要確保此次異動沒有產生新的錯誤，當然既有的測試也不能產生新的錯誤。
7. 追加必要的測試
一樣假定我們正在處理的 code base 都有自動化測試，那麼請為你這次的 PR 撰寫新的測試。
在我收過得 PR 裡沒有包含測試這種情況並不算常見，但當我收到這類型的 PR 時，我會把它 reject 掉。
這不是什麼很困難遵守的規範。有很多種情境下你在新增 code 的時候不需要補上測試 (像是新增的 code 是 Humble Object)，但是當 code 是可以被測試的時候，請為它增添測試。
你可能會需要遵循 code base 既有的測試寫法。
譯者個人想法：這個目前(2021 年)在職場上依舊屬於有的話很好，後面補上也不錯的狀況。是有一些開發團隊可以嚴格遵守這個規範，但是在 time to market 的神聖律法之下，是不太可能接受因為還要補上測試所以尚未出 build 的事情發生。很多情況下會是開發者自行在本機運行來檢視 code 是否符合需求。另外一個要作到這個要點的困難之處，就是測試案例通常不會在盤點需求時很快的被列出，而列出案例通常也是滿花時間的一環，這也導致在專案時程比較吃緊的時候測試通常會是被犧牲掉的部份，因為在職場裡通常會有一個職位叫 QA 他們會被要求默默吃下這個責任 (不用寫 Unit test(單元測試)但卻對 code 的品質負責)。
8. 寫下異動的原因
能自我描述的 code 實在相當罕見。
沒錯，註解就是在道歉，而我當然是比較偏好妥善命名的函式、類別與變數。當在撰寫程式碼時，你依然要時常要為那些不言自明的事情決定名稱(特別是處理商業邏輯時)。
寫下為什麼 你會這樣寫，而不是這段 code 在做什麼
我偏好的優先序如下：
自我描述的 code： 你可以做一些關於 code 自我描述的決定。Clean Code 就是一本實實在在的闡述如何作到這點的一本書。
code 註解： 若你不能讓 code 充分的自我描述，那就加上 code 註解吧。至少註解是寫在 code 旁邊的，所以即使你打算換版控(version control system) 這種不太可能發生的狀況發生了，註解依然會保留下來。這是我找到一個比起以我構思解決問題的方法，註解更適合的範例。
commit 訊息： 大多數的 version control system 能讓你寫下 commit 訊息。多數人除了最基本的要求沒有煩惱要寫什麼內容，但你也可以在這邊寫下你的異動原因。有時候你要有條理的寫下你為什麼這樣做的理由。這些事情並不適合寫在 code 註解上，但很適合寫在 commite 訊息裡。但這些資訊會因為你更換 version control system，而完全的從 source code 中失去。這裡是一個我覺得有需要寫大量的 commite 訊息的案例，但我通常不會寫這麼多。
PR 註解： 在很罕見的機會裡，你可能身處上述這些項目通通不適當的情況。在 PR 管理系統像是 GitHub 或 Stash，你可以自己在 PR 加上客製的訊息。這個訊息可視為 source code 的近親，只要你待在同一個平台就會一直存在。但若你移往不同的平台像是 CodePlex 搬到 GitHub 你就會失去所有的 PR 註解。依然是不太常見的情況，我需要對 reviewer 解釋，但不管用什麼角度來解釋都要用到 source code 之外的東西。這個是其中一個例子。
這些看來是理所當然的事情，但請小心謹慎。那些你視為理所當然的事情可能別人並不這麼認為，或是三個月後的你。
譯者個人想法：
關於「註解就是在道歉」這段話，這邊貼上一段 Uncle Bob 的一段原文：
A comment is an apology for not choosing a more clear name, or a more reasonable set of parameters, or for the failure to use explanatory variables and explanatory functions. Apologies for making the code unmaintainable, apologies for not using well-known algorithms, apologies for writing ‘clever’ code, apologies for not having a good version control system, apologies for not having finished the job of writing the code, or for leaving vulnerabilities or flaws in the code, apologies for hand-optimizing C code in ugly ways. And documentation comments are no better. In fact, I have my doubts about docstrings.
by Uncle Bob認真講 Uncle Bob 真的滿嚴格的。
而 commit 訊息是我在目前的職涯中比較少看到有人在認真經營的一塊，不過當你看過有在好好處理 commit 訊息的例子的話，你會感受到這些帶來的好處，尤其是當你要追朔為何那段 code 會被那樣改時，只看 code 只會知道為何要那樣寫但是缺乏脈絡，這種事情需要 version control system 來做追蹤比較有效率也比較直覺。關於怎麼寫出一個具有足夠表達力的 commit 訊息可以好好參考這裡 (約定式提交)，以及這裡 (How to Write a Git Commit Message)。
至於 PR 註解真的是除了 open source 之外就看開發團隊的習慣了，在職場環境中若能妥善利用 PR 管理系統中的註解功能，來針對異動做出討論的話，對於異動的思路探討、決策脈絡的理解都會很有幫助，但這點一樣…在我所經歷過得職場中，很少會有開發團隊善用這功能，這是比較可惜的地方，但可能也只是臺灣目前這種風氣還不盛行，主要還是習慣在任務分派系統中紀錄討論的內容(像是 JIRA、Trello、Asana 等)。
9. 好好寫
把 code 寫好，也要把文寫好。這個部份是有點主觀的，但有一些準則同時適用在 code 與 文字。Code 有一套明確規則；若觸犯的話你連 compile 都會失敗 (用白話文講，就是你在執行時期就會失敗)。
在書寫時你可能也會這些事加上一些準則；code 註解、commit 訊息、PR 訊息。
請用正確的拼字、文法以及標點符號。若你沒有這樣做，你的文將會難以理解，而 reviewer 可是正常人。
譯者個人想法：這點比較像規勸大家既然要寫就不要在註解或是訊息裡面裝可愛或是賣弄文學，盡可能寫下有條理的字句不管那是 code 還是段註解。一樣，這一項也是相當認同，畢竟看 code 的時間總是遠大於寫 code 的時間，你不會想讓自己在一段 code 或是一段話用揣測的方式理解，當然，除非這是一段寫後即焚的 code (有的話請跟我說，我目前想像不出來這什麼情境)。
10. 避免 commit 跳針
有些時候 reviewer 會指出 PR 裡數個問題，而且你也同意修改。
這會導致你追加一些 commit 在你的 PR 分支上。這基本上沒什麼錯，但這可能會導致一些不太合理的 commit。
舉例來說，你的 commit 包含五個 commit：[A,B,C,D,E]。 Reviewer 並不喜歡你的 commit B 和 C，所以他要求你移除那段 code。大多數的人都會 checkout 發 PR 的那個 branch 然後刪除那段指定的 code 並產生新的 commit F 到 commit 清單中，所以清單就變成 [A,B,C,D,E,F]。
你為什麼要把加了一段 code 又把它移除的一組 commit 給 merge 進來呢？這就只是一組沒有意義的 commit，並沒有任何價值在裡面。
你應該要做的是，移除個引起問題的 commit，並強制 push 到你改的那條 branch，讓 commit 變成 [A,D,E]。因為你是 branch 的擁有者，所以你可以依你的意思修改並強制 push。
另外一種我很常見的抖動是當一個 PR 太古老時(通常是因為冗長討論)：在這種情境下，發 PR 的人會定期的把主幹(master branch)合併進 PR branch。
一樣的道理，為什麼我要看這些 merge commit？你是這個 branch 的擁有者，就乾脆一點 rebase 你的 PR branch 並強制 push 上去。這樣 commit 歷程才會簡潔。
譯者個人想法：整理 commit 這種行為在我到目前為止的職場環境中真的很少見到，而願意做的人通常有某種程度的潔癖，但不可否認的是整理完後的 commit 真的讓人看的神清氣爽，眼睛跟心情會舒服很多。我自己其實也鮮少用 rebase 來整理 commit，除非發生上述那種 code 反覆出現又刪除的狀況。通常都是拿來跟 master branch 做 sync 用；也就是減少 PR 身上的 merge commit，因為我也不喜歡在 PR 身上看到那種 commit，那些 commit 以 PR 來說真的是毫無價值又會混淆該審閱的內容。不過回過頭來講這講這件事吧，要整理 commit 是真的非常花時間，尤其是當你作到這個動作時，通常需求的範圍你已經開發完，在有時程的壓力下，業界除了犧牲單元測試外，這種整理性質的動作更是毫無例外的第一個被犧牲，而且更難爭取與說服為什麼要花這些時間幹這種沒有商業價值的事情。這種整理習慣真的是要對自我有要求才會去做，而且還不能影響日常開發速度，不然肯定是會被抓起來拉正的。
結語
你的 PR 會被一個或多個 reviewer 審核。不要增加你的 reviewer 額外的工作。
你讓 reviewer 有越多事要做，你的 PR 就有越高的風險被拒絕。