關於pull request這檔事

之前寫過關於code review這檔事，那反過來說，關於發pull request這檔事呢？(或者說發patch也適用，以下就姑且都說PR吧，節省篇幅愛地球。)

Automattic的夥伴們都挺愛挑毛病修細節的，PR來回修個幾週才進、打掉重寫，或是要拆細重發等等多不勝數，所以以下也算是個人在敝公司發PR的生存守則：

1. 單一目的
2. 測試方法清楚簡單
3. 迭代大於完美

1. 單一目的

白話一點說，就是一個PR只做一件事，而且要在PR的summary處描述清楚。所謂『只做一件事』有點模糊，畢竟每個人的標準不太一樣，而且切得過細也不見得是好事，失去大方向可能會讓reviewer看得一頭霧水。那怎樣的大小叫剛好？我覺得較通用的原則就是能切出來獨立運作的就切，這樣通常就不會太大。

舉例來說，在開發calypso的時候，如果我們要加一個頁面，而該頁面會與某service API溝通，常見的做法是切成至少：

• app state一個，也就是redux reducer / selector / action creators
• API溝通一個，也就是data layer handler
• UI一個，視UI複雜度可能會拆細成components PRs數個與最後組合成頁面的PR

2. 測試方法清楚簡單

自己寫的程式碼，自己理當要是最清楚怎麼測試的人，而不是要reviewer發揮創意幫你想辦法測試。應當要由自己提供清楚、step by step就能完成基本正確性驗證的測試，這時再由reviewer去發揮創意想沒涵蓋到的地方。基本原則是所有code path都要跑到，當程式碼是JavaScript或PHP這種語言的時候這特別重要，畢竟compiler不會幫你檢查沒跑過的地方是不是根本就是一坨XX。

至於什麼樣叫『簡單』呢？應該能自動化就自動化，不能自動化的部分就要指示清楚明瞭。data flow的部分幾乎都能unit test所以就該寫unit test、React component可以用enzyme或jest snapshots、service APIs的部分可以用來確定各種API response下的行為、實際與service API溝通的部分可以寫個script發真的request並檢查所有response是不是都和預期相符、end to end tests比較麻煩但還是可以做，例如wp-e2e-tests。

以上節範例來說，app state和data layer handler的部分都很容易可以unit tests，那就應該要力求一行npm run test-client就結束。UI的部分不見得會寫unit tests，但一定要附上詳細的使用流程，力求讓reviewer無腦地跟著指示就能用、就要知道預期的結果為何。

3. 迭代大於完美

最後，與其等一切都臻於完美再發PR，不如一開動就發，標上work in progress，再逐步迭代。個人經驗上，這在參與開源專案或遠端工作都很有幫助；公開透明、勤於溝通，是這兩者共通的生存必備技能。我過去曾有幾次自己埋首寫了一週，結果一丟出來馬上就被整個翻掉。這不但對自己的士氣是一大打擊，也會拖累整個團隊。而且，這與其說是追求完美，不如說單純只是怕丟臉罷了。

儘早公開你在做的事，不但讓同組的人可以第一時間參與討論，及早修正方向，也常常可以釣那些閒閒沒事逛PR當小說在看的人進來幫忙。