More than 5 years have passed since last update.

freee EngineersAdvent Calendar 2015

@nasa4w(n asa)

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

Posted at 2015-12-09

こんにちは、freee でエンジニアをしている @nasa4w です。
この記事は freee Engineers Advent Calendar の10日目です。

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

まとめ - 今回作ったブランチとPR -

長くなってしまったので結論をさきに置いておきます。PRを複数に分割する場合のゴールはこんな感じ。

Why?（なぜこうしたか）

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

詳細

背景

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

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

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

問題

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

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

このままだと、

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

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

解決策を考える

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

PRの分割と同時に実現したかったこと

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

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

構成

その結果、**「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

初期状態
- 青（）の commit が変更内容

start 地点をいまの最新の develop にする

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

PRを２つにわけるため、workブランチを元に2つのブランチを作る

見やすくするために、upstream:developを基準に左寄せしてます

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 だけをなかったコトにする

workブランチを作りなおす

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

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 の基準よりもすすんでいるため)

2つのブランチをpushして、GitHub上でPRをつくる

# 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つにわけれるようになったよ！！٩(๑❛ᴗ❛๑)۶ ﾔｯﾀ～

Review指摘があって直すときには（例： view の修正の場合）

# 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

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

さいごに

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

workで試行錯誤
commitしたい分を git stash
topic_view or topic_logic へ git apply
綺麗にした work にcommitしたブランチを git merge

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

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

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

宣伝

freeeではサービスから基盤、そして必要となればモバイルアプリまで縦横無尽に踊りまわるフルスタックエンジニア: Eng を募集しています。よろしくお願いします。

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

参考資料

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

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

You get articles that match your needs
You can efficiently read back useful information
You can use dark theme

What you can do with signing up