レビューしやすいPRの作り方 - 大きいPRを複数に分割する -

こんにちは、freee でエンジニアをしている @nasa4w です。
この記事は [freee Engineers Advent Calendar](http://qiita.com/advent-calendar/2015/freee) の10日目です。

今回は大きな機能を複数のPRに分割する話をします。
freeeでも絶賛模索中の取り組みなので現在進行形の話ですm(_ _)m

# まとめ - 今回作ったブランチとPR -
長くなってしまったので結論をさきに置いておきます。PRを複数に分割する場合のゴールはこんな感じ。
![全体像.png](https://qiita-image-store.s3.amazonaws.com/0/50637/064f9a6f-dab0-b2cf-e2ce-11b4b48ba1e2.png)

## Why?（なぜこうしたか）
* こうすることで、常に work ブランチを使って動作確認ができる
* レビューする PR を２つに分割できる
  * view の PR と logic の PR
* レビュー指摘の反映は、topic_view, topic_logic ブランチに commit すればOK
  * work ブランチにはこの2つのブランチを merge しなおすだけでOK
* PR をたどってレビュー指摘が GitHub 上で追える


# 詳細 
## 背景

freee には会計をはじめ4つのサービスがあり、これらのサービスで共通して利用する機能を基盤に実装しています。私はふだんその基盤とよばれるところを開発しています。
アプリと基盤両方に変更が入るような大きな機能追加・変更の場合、クロスファンクショナルなチームをつくって進めています。
これまでにそうして関わった複数のプロジェクトで、たびたび以下のような声をきいてきました[^0]。

「大きな PR、いつ、どれくらいの大きさでレビューが飛んで来るかわからなくてつらいっす」
「大きい PR 読みづらいです…」

そこで、
今回のプロジェクトではあらかじめレビュワーのエンジニアと相談して、
PRを "view" と "logic" の2つに分けてだすことにしました[^1],[^2]。

## 問題
PRを2つにわけることにしたはいいものの、開発をはじめていきなり問題に直面しました。
「PRをわけるとして、開発時の動作確認どうしよう…」

私がローカルで開発をするとき、動かしながら開発をすすめることが多いです。
単純に、
「PR を logic と view にわけるなら、それぞれの変更をあらかじめ logic 用ブランチ、view 用ブランチに分けてコミットしながら作業しよう〜」
と考えて開発をはじめようとして、はたと気づいたわけです。

このままだと、

* **コミットするときにいちいちブランチ切り替える必要があってめんどくさい**
* **絶対ブランチを間違って、逆にコミットする自信がある…！**
* 動作確認したいときには、毎回動作確認用ブランチに切り替えて 2つのブランチを merge して確認する…のか……？とても…面倒です……

そんなわけで、
とりあえずはローカルの1つのブランチに全部ごった煮でコミットして開発し続けました。

## 解決策を考える
いよいよPRをつくってレビューのタイミングとなったので、
レビュワーと再度相談して決めたのが上のまとめに書いた構成です。

### PRの分割と同時に実現したかったこと
レビューを楽にするために、PRは分割したい。
でも、かといって開発を面倒にしたくないね、ということでPRの分割と同時に実現したいことを決めました。
それがこちら。

1. **レビューする内容を view と logic の2つのPRにわける**
1. **常に動作確認できるブランチがあるとよい**
    * PRをわけるにしても手元での動作確認のためには、そのたびに2つのPRのmergeを行う必要があります。できればそういった作業は省略できたら素敵です。
1. **upstreamへのmerge後に、GitHub上でレビュー指摘内容を追えるようにしたい**
    * 大きな機能の場合、あとから他の人が見た時に指摘内容を確認できることは重要です。
1. **レビュー指摘の反映を楽に動作確認用ブランチに反映したい**
    * レビュー時も開発スピードを落とさずスムーズに反映したい。動作確認用ブランチに簡単にコミットを反映したい！


### 構成
その結果、**「3つのPR」****「4つのブランチ」**で作業をすすめることになりました。

* PR
    * 2つは自分のリポジトリへのPR
    * 1つはupstreamへのPR
* ブランチ
    * upstream:developへのmerge用…*new_function*ブランチ
    * viewのレビュー用…*topic_view*ブランチ
    * logicのレビュー用…*topic_logic*ブランチ
    * 動作確認用…*work*ブランチ

## How to create branches and PRs
1. 初期状態
    * 青（ :blue_book: ）の commit が変更内容

    ![初期状態.png](https://qiita-image-store.s3.amazonaws.com/0/50637/8b21d76a-6a11-b057-6bd2-94ee6cbbe663.png)

1. start 地点をいまの最新の develop にする
![startを今の最新のdevelopに.png](https://qiita-image-store.s3.amazonaws.com/0/50637/48c96876-b575-50ea-c3f0-c0310bc51857.png)

    ```
    git checkout tmp              # 開発中のブランチ tmp に移動
    git checkout -b work          # 同じ内容で新しいブランチを作っておく（操作間違ってもいいように）

    git checkout develop
    git pull upstream develop     # develop を最新にする
    git checkout work             # work に移動
    git rebase develop            # work を develop から rebase 
    ```

1. PRを２つにわけるため、workブランチを元に2つのブランチを作る
    * 見やすくするために、upstream:developを基準に左寄せしてます
 
    ![PRを分けるためにブランチを用意.png](https://qiita-image-store.s3.amazonaws.com/0/50637/4f1499e4-f220-ec46-f614-403fc79d2738.png)

    ```
    git checkout work                  # work ブランチを元にする
    git log --decorate                 # 起点にしたいコミットの hash 値を確認

    # ここからの作り方は人ごとに違うはず(両方 rebase -i でやるのが普通かも…)
    git checkout work
    git checkout -b topic_view         # work ブランチを元に topic_view をつくる
    git rebase -i <hash値>             # view の commit （4以外）の行を消して保存・終了

    git checkout work                  
    git checkout -b topic_logic        # work ブランチを元に topic_logic をつくる
    git reset --hard HEAD~             # view の commit だけをなかったコトにする
    ```

1. workブランチを作りなおす
    * この作業をしておくと、*topic_view*, *topic_logic* ブランチへcommitした修正を *work* ブランチに反映しやすくなる

    ![workブランチを作りなおす.png](https://qiita-image-store.s3.amazonaws.com/0/50637/3a3d7aa2-b420-18f6-6106-0a871597f3e6.png)

    ```
    git checkout work
    git reset --hard <hash値>        # work ブランチをリセット
    git merge topic_logic            # topic_logic を work に merge (fast-fowardされる)
    git merge topic_view             # merge commit が作られる 
                                     # (work にはすでに mergeによって1,2,3が commit されていて、
                                     #  topic_view の基準よりもすすんでいるため)
    ```

1. 2つのブランチをpushして、GitHub上でPRをつくる
  ![PRを出す.png](https://qiita-image-store.s3.amazonaws.com/0/50637/3624f704-0bbd-895a-b38c-77ced789d66e.png)

    ```
    # upstream:develop に PR をだすためのブランチをつくる
    git checkout develop
    git checkout -b new_function

    # GitHub 上の nasa のリポジトリ（origin）にブランチを作っておく
    git push origin new_function 

    # PR を出したいブランチを push
    git push origin topic_view
    git push origin topic_logic 

    # いまpushした2つのブランチから、
    # GitHub 上で origin:new_function に向けてPRをつくる
    ```

     **ここまでで PR 2つにわけれるようになったよ！！٩(๑❛ᴗ❛๑)۶ ﾔｯﾀ～**

1. Review指摘があって直すときには（例： view の修正の場合）
![review時の流れ.png](https://qiita-image-store.s3.amazonaws.com/0/50637/da6ff5e9-4de5-d9f0-3b63-c8ebf3c0f943.png)

    ```
    # view に修正が入ったら view に commit して、work に topic_view を merge
    git checkout topic_view
    git commit
    git push origin topic_view

    # work に merge すれば、常に work を使って動作確認できる
    git checkout work
    git merge topic_view
    ```

1. [review終了後] upstream:develop に PR をだす
   * レビューが終わったら *topic_view*, *topic_logic* から *new_function* にでてる PR を GitHub 上で merge
   * upstream:develop に origin:new_function からPRを作成
      * **この図の緑 :green_apple: の PR が普段の開発でつくってる PR に相当**

   ![C-FOへのPR.png](https://qiita-image-store.s3.amazonaws.com/0/50637/60175dca-980d-e519-ef49-a4a567caad47.png)

---

# さいごに
ここまでお付き合いいただきありがとうございました。
現在この仕組みで開発中ですが、Review指摘の反映などで追加変更する場合は以下の流れでやってます。

1. workで試行錯誤
2. commitしたい分を git stash
3. *topic_view* or *topic_logic* へ `git apply`
4. 綺麗にした *work* にcommitしたブランチを *git merge*

*work* は動作確認用のブランチと同時に、テンポラリな開発用ブランチとしても使ってます。
このテンポラリな開発用ブランチは実際使ってみるととても楽で、動作確認するときには分割した2つのブランチをmergeするだけで抜けもれなく変更がはいっていることが確認できる（そして、upstreamへのPRはこの2つのPRがmergeされたものから作成される！）点が助かっています。

冒頭に書いたように、
freee でも試行錯誤中の話なのでまだ振りかえって共有できる学びが少ないのが心苦しいのですが少しでも役に立てば幸いです。

引き続き大きな機能でも開発してる人にもレビュワーにも負担のかからない開発方法を模索していく所存です！



# 宣伝
freeeではサービスから基盤、そして必要となればモバイルアプリまで縦横無尽に踊りまわる [フルスタックエンジニア: Eng](http://jobs.jobvite.com/freee/job/oYto0fwz) を募集しています。よろしくお願いします。

明日はfreeeの初代フロントエンドヤクザの @yo_waka です。お楽しみに！

# 参考資料
* 『こわくない Git』 http://www.slideshare.net/kotas/git-15276118
   * 上の絵もすこし↑を意識して書いてます（e.g. 青丸の M は merge-commit）

---
[^0]: 今回の問題はクロスファンクショナルなチームだから発生するわけではないですが、クロスファンクショナルなチームだと起きがちな気がします。
[^1]: レビュワーが決まっている場合は、どうPRをだすのがいいか、レビュワーの好みに合わせるのがいいと思います。
[^2]: 追加する機能ごとにPRを分けるのも方法の1つです。ただ、基本的に1つの機能で完結しない事が多いかと思います（例：ある設定変更のそうさであれば、 "適用できる状態か？の確認", "設定", "設定を取り消す" などの操作はそれぞれ他の機能なしには全体の整合性を見づらいですよね）。最終的に全体を見ることを考えると、機能単位でPRがでることで見やすくなるかと言われると……たぶんレビュー時にPR間の移動が発生してあんまり見やすくない気がしています。