Hatena::ブログ(Diary)

hnwの日記 このページをアンテナに追加 RSSフィード

[プロフィール]
 | 

2011年5月28日(土) GitHubへpull requestする際のベストプラクティス このエントリーを含むブックマーク このエントリーのブックマークコメント

みなさん、Git使ってますか?僕はまだメインのVCSSubversionなのもあって、なかなか慣れません。せっかくGitを使っているのに、ちょっと不便なSubversionくらいの位置づけです。でも、同じような理解度の人って多いんじゃないでしょうか。


一方で、最近はGitHub管理のオープンソースプロジェクトが増えてきました。バグレポートを送るにしてもpull request*1が前提のような空気があり、Git初心者には少し敷居が高い印象があります。


そんな僕も先日初pull requestをしてみたんですが、色々な失敗の積み重ねで残念なpull requestになってしまいました。その反省を元に、本稿ではpull requestする際のベストプラクティスを紹介します。これは「Git Workflow」をベースにコマンド例などを加筆したものです。


概要

pull requestする際は、次の流れのようにすれば万全です。


  1. GitHubforkします。
  2. できたforkをローカルにcloneします。
    $ git clone git@github.com:hnw/Spoon-Knife.git
    
  3. 作業ディレクトリに移動します。
    $ cd Spoon-Knife
    
  4. 作業用ブランチで作業します(あとで説明します)
  5. fork元の更新に追随します(あとで説明します)
  6. commitを1つにまとめます(あとで説明します)
  7. GitHubにブランチをpushします。
    $ git push origin myFeature
    
  8. GitHubからpull requestを送ります。

ここで大事な点を強調しておきますが、絶対にmasterブランチで作業してはいけません。また、masterブランチからpull requestを送るのもいけません。必ずpull requestのための別ブランチから送るようにしましょう。これにはいくつか理由があるのですが、順を追って説明していきます。


作業用にspike/prototypeブランチを作る

上記の手順4について説明します。手順7でpull request用のローカルブランチをpushしていますが、いきなりpull request用のブランチで作業するのではなく、作業用にさらに別のブランチを作ることをお勧めします。簡単な修正をするつもりでも、試行錯誤して何度もcommitしたり、最初からやり直したりするかもしれません。そのための作業用ブランチを別に切っておくというわけです。


作業用ブランチには、spike(XP用語、お試し実装といった意味だと理解しています)とかprototypeとかいう単語を入れておくと、GitHubを見ている他の人にもブランチの意図がわかりやすくて良いでしょう。


$ git checkout -b myFeatureSpike
Switched to a new branch 'myFeatureSpike'
$

では、ファイルを編集して次のようにローカルリポジトリにcommitを行ったとします。


$ vi README
$ git commit -a -m '感想を追記'
[myFeatureSpike 12ca2a5] 感想を追記
 1 files changed, 3 insertions(+), 1 deletions(-)
$ vi README
$ git commit -a -m 'コナミコマンドについてREADMEに追記(ネタバレ)'
[myFeatureSpike bd66e17] コナミコマンドについてREADMEに追記(ネタバレ)
 1 files changed, 2 insertions(+), 0 deletions(-)
$

一通りローカルでの作業が済んだらpushしましょう。ローカルのmyFeatureSpikeブランチの内容を同名のリモートブランチとしてpushします。


$ git push origin myFeatureSpike
Counting objects: 8, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (6/6), done.
Writing objects: 100% (6/6), 789 bytes, done.
Total 6 (delta 1), reused 0 (delta 0)
To git@github.com:hnw/Spoon-Knife.git
 * [new branch]      myFeatureSpike -> myFeatureSpike
$

fork元の更新に追随する

作業中にfork元のリポジトリが更新されることがあるかもしれません。これに追随するための手順は次のようになります。


  1. fork元リポジトリをupstreamという名前で登録(最初の1回だけ)
    $ git remote add upstream git://github.com/octocat/Spoon-Knife.git
    $
    
  2. commit前の差分があるならstashする
    $ git stash
    Saved working directory and index state WIP on myFeatureSpike: bd66e17 コナミコマンドについてREADMEに追記(ネタバレ)
    HEAD is now at bd66e17 コナミコマンドについてREADMEに追記(ネタバレ)
    $
    
  3. masterブランチに移動
    $ git checkout master
    Switched to branch 'master'
    $
    
  4. ローカルのmasterブランチの内容をupstream/masterに同期させる
    $ git pull upstream master
    From git://github.com/octocat/Spoon-Knife
     * branch            master     -> FETCH_HEAD
    (略)
    $
    
  5. 作業ブランチに移動
    $ git checkout myFeatureSpike
    Switched to branch 'myFeatureSpike'
    $
    
  6. fork元の最新版までrebaseする(コンフリクトしたら適宜がんばる)
    $ git rebase master myFeatureSpike
    First, rewinding head to replay your work on top of it...
    Applying: 感想を追記
    Using index info to reconstruct a base tree...
    Falling back to patching base and 3-way merge...
    Auto-merging README
    Applying: コナミコマンドについてREADMEに追記(ネタバレ)
    Using index info to reconstruct a base tree...
    Falling back to patching base and 3-way merge...
    Auto-merging README
    $
    
  7. 同期・rebaseした分をGitHubにpushする
    $ git push origin master
    (略)
    $ git push -f origin myFeatureSpike
    (略)
    $
    
    • spikeブランチが既にpushされている場合はfast-forwardでなくなるため-fは必須です。
    • 公開状態の履歴を書き換えることになるため褒められたことではありませんが、spikeと名乗っているので平気なことにします。
    • -fを避けるためにさらに別のブランチを切ることもできますが、自分自身が混乱しそうなので僕ならやりません。
  8. stashした分を元に戻して作業を継続(コンフリクトしたら適宜がんばる)
    $ git checkout myFeatureSpike
    Switched to branch 'myFeatureSpike'
    $ git stash pop
    (略)
    $
    

上記フローでは、masterブランチをfork元への追随のために利用しています。これがmasterブランチ上で作業をしない最大の理由だと言えるでしょう。


この作業をしなくてもpull requestすることはできますが、万一最新版とコンフリクトするような場合は対処を中の人に任せることになってしまいます。中の人は忙しいことも多いでしょうから、できるだけ最新版までrebaseしてからpull requestすべきです。


pull request用に過去のcommitを1つにまとめる

さて、ついに作業ブランチ上に満足のいくファイルをcommitできたとしましょう。fork元の更新にも追随できています。早速pull requestだ!と言いたいところですが、その前に今までの作業を1commitにまとめたブランチを作り直しましょう。


というのも、作業ブランチ上では試行錯誤したりfork元に追随したりした跡が複数commitに分かれているはずです。これらをそのまま本家プロジェクトでpullしてしてしまうと、複数commit全てが本家の履歴として残ってしまいます。後で差分をチェックする人のためにも、あらかじめ整理してからpull requestすべきでしょう。これは次のような作業になります。


  1. 作業ブランチに移動
    $ git checkout myFeatureSpike
    Switched to branch 'myFeatureSpike'
    $
    
  2. 作業ブランチを元にpull request用のブランチを作成・移動
    $ git checkout -b myFeature
    Switched to branch 'myFeature'
    $
    
  3. 作業ブランチ上のfork元からの差分を1 commitにまとめる
    $ git rebase -i master
    

git rebase -iで全差分を1 commitにまとめるには、2個目以降のcommitを全部squash指定します。たとえば次のような内容がエディタに表示されたとしましょう。


pick 12ca2a5 感想を追記
pick bd66e17 コナミコマンドについてREADMEに追記(ネタバレ)

# Rebase bdd3996..bd66e17 onto bdd3996
#
# Commands:
#  p, pick = use commit
#  r, reword = use commit, but edit the commit message
#  e, edit = use commit, but stop for amending
#  s, squash = use commit, but meld into previous commit
#  f, fixup = like "squash", but discard this commit's log message
#  x, exec = run command (the rest of the line) using shell
#
# If you remove a line here THAT COMMIT WILL BE LOST.
# However, if you remove everything, the rebase will be aborted.
#

このような場合、次のように修正してセーブ・終了します。


pick 12ca2a5 感想を追記
squash bd66e17 コナミコマンドについてREADMEに追記(ネタバレ)

# Rebase bdd3996..bd66e17 onto bdd3996
#
# Commands:
#  p, pick = use commit
#  r, reword = use commit, but edit the commit message
#  e, edit = use commit, but stop for amending
#  s, squash = use commit, but meld into previous commit
#  f, fixup = like "squash", but discard this commit's log message
#  x, exec = run command (the rest of the line) using shell
#
# If you remove a line here THAT COMMIT WILL BE LOST.
# However, if you remove everything, the rebase will be aborted.
#

すると次に2個分を1個にまとめたcommitのログを書くよう求められますので、適当に書いてセーブしましょう。これでmyFeatureSpike上の複数commitがmyFeatureブランチ上では1 commitにまとまった状態になりました。


pull requestする

fork元の最新状態に追随できていていますか?1 commitにまとめましたか?では、いよいよpull requestしましょう。

  1. pull request用のブランチをGitHubにpush
    $ git push origin myFeature
    (略)
    $
    
  2. GitHubからpull requestします。GitHubUIから適切なブランチを選びましょう。
  3. 本体に取り込まれるのを待ちます。「コーディング標準に合わせろ」「テスト書け」などの文句が来たら別途commitしましょう(できれば最初からやっておきましょう)。

pull requestはGitHubのissue tracking systemと同期しており、pull requestと同時にチケットが起票されます。ふだんのバグレポートと同様に、問題点とパッチの概要などを簡単に書けばよいでしょう。


1点だけ、pull request後の注意点があります。pull requestに使ったブランチで別の作業をしてはいけません。万一無関係なcommitをpushしてしまうと、pull requestにも反映されてしまいます。


まとめ

masterブランチからのpull requestが許されるのは小学生までです。僕の失敗談を紹介すると、あるプロジェクトに2つの修正をそれぞれpull requestするつもりだったのが、2個ともmasterで作業していたため、2つの修正が混ざったカオスなpull requestを投げてしまいました。ホントに迷惑すぎですね。


今回の手順に従うと、かなりGitっぽい作業ができるはずです。というより、上記はGitで協調作業するときの流れをpull requestに合わせて説明したものだと言えます(originとupstreamとでremoteが2個あるため、通常のパターンよりも複雑ですが)。


面倒すぎるだろ!と感じた人がいるかもしれません。特にブランチ周りはGit初級者がつまずきやすい場所だという印象がありますが、Subversionとの違いがわかればじきに慣れると思います。チンプンカンプンな方はPro Gitの「ブランチとは」あたりを読み進めてみてください。


僕は必要にならないと覚えない性質なので、今回pull requestしてみてGitの理解が随分進んだ気がします。皆さんもpull requestのついでにGitの知識を深めてみませんか?

*1:自分のGitHubへのcommitをそのまま本家に取り込んでよ、というリクエスト。BTSへの登録も同時に行われる。GitHub用語。

*2:これはforkのデモ用リポジトリで、GitHubのドキュメント「Fork A Repo」の中で紹介されています。GitHubfork数ランキング第3位(2011/5/28日現在)という大人気プロジェクトです。

月 2011/05/28 21:25 記事にまとめる作業、おつかれさまでした。
pull request際には参考にさせていただきます。

月 2011/05/28 21:26 > pull request際には

pull requestする際には

bleis-tiftbleis-tift 2011/05/30 10:57 masterブランチで作業するのと、masterブランチからpull requestを送るのは分けて考えた方がいいのではないでしょうか?
masterブランチで作業を行わないようにすれば、masterブランチ上で作業が混ざることはありえないので、masterブランチからpull requestを送っても何の問題も無いように思えます。

hnwhnw 2011/05/30 14:47 bleis-tiftさん:
おっしゃっている状況がイマイチわからないのですが、1個目のpull requestが採用されるより前に2個目のpull requestを出したいような場合は、それぞれ別ブランチを利用するしかありません。(そうでないと、1個目のpull requestに影響を与えてしまうはずです)。1個目はmasterからpull requestして2個目以降を出したいときだけ別ブランチを切るようなルールにしてもいいと思いますが、それよりは最初から「masterブランチからはpull requestしない」と決めてしまった方がわかりやすいように僕は感じました。

masterからpull requestするのは小学生だけ、という文言が気に障ったようならごめんなさい。ただ、たくさんpull requestするような人は大抵ブランチから送っているように思います。

hnwhnw 2011/06/06 23:20 今更ですが追記します。masterに自分の修正を入れてしまうと、upstreamの追随にmasterが使えなくなります。upstream追随用のbranchを別途作るよりは、upstream追随用にmasterを使う方針の方がわかりやすい気がします。

現時点の僕がいいと思った案を紹介しているだけですので、より良いアイデアがあれば教えてください。

bleis-tiftbleis-tift 2011/06/10 15:09 > masterからpull requestするのは小学生だけ、という文言が気に障ったようならごめんなさい。

あ、いえ。そういうつもりはないです。

> 1個目のpull requestが採用されるより前に2個目のpull requestを出したいような場合

そこまで頻繁に pull request を投げ、かつそれがほとんど採用されるような状況なら、commit 権限をもらった方がどちらも幸せになれるんじゃないかな、とか。
頻繁に pull request は投げるけどほとんど採用されないような状況なら、言い方は悪いけど足枷になっちゃってると思うんですよね。
pull request を投げる方針もベストプラクティスを名乗るなら必要なのかな、とかなんとか。

> masterに自分の修正を入れてしまうと、upstreamの追随にmasterが使えなくなります。

これは確かにそうですね。
そんなに頻繁に pull request しない場合は master でもいいじゃん、と思ってましたが、これからはそれ用のブランチからやるようにしようと思います。

hnwhnw 2011/06/10 17:01 なるほど、「どういうシチュエーションで必要なのか」の共有がないと伝わるものも伝わらないですよね。上の文章で真っ先に書くべきところでした。ごめんなさい。

僕の場合についてもう少し詳しく書くと、GitHub上で公開されているライブラリを試用していたところ、数時間のうちに独立したバグを2個見つけてしまったんですよね。そこでpull requestを2個投げようとしてアワアワしたというわけです。これは他の人にも十分ありうるケースのように思いますし、この段階でcommit権限をもらうという選択肢は無いと思います(まだ数時間しか使っていないですから)。

また、そうでなくてもcommit権限をもらうのは一定のハードルがあるはずですから、中の人に実力と理解度を認められるまでは上のフローで作業することになると思います。

bleis-tiftbleis-tift 2011/06/10 19:38 > GitHub上で公開されているライブラリを試用していたところ、数時間のうちに独立したバグを2個見つけてしまったんですよね。そこでpull requestを2個投げようとしてアワアワしたというわけです。

この場合、pull requestは一つにまとめてしまうという選択肢もありますよね。
例えば一つのバグ修正しか採用されないにしても、cherry-pickやrebase -iがあるので手間はかかりませんし、どちらも採用する場合はまぁ普通に処理する感じで。

機能追加しててそれに関係するバグを見つけてしまって・・・という場合はまずバグを追加して、そのうえで機能追加したブランチをバグ修正したブランチにrebaseしてpull request投げる、とか。
そうすればバグ修正は取り込むけどその機能追加は取り込まないよ、という場合はやはりcherry-pick/rebase -iを使えばいいですし、この状況で機能追加だけ取り込むことはしないでしょうし。

トピックブランチ一個につきpull requestって運用はどうなんだろうなぁ、という思いがあります。
もうちょっとまとめて送ってくれた方が嬉しいような、そうでもないような・・・難しい。

> commit権限をもらうのは一定のハードルがあるはず

はい。なので「頻繁に pull request を投げ、かつそれがほとんど採用されるような状況」と書きました。

hnwhnw 2011/06/10 20:19 pull requestを一つにまとめてしまうと、GitHubのissue tracking system上でも「修正Aと修正B」というようなチケットが立ってしまいますよね。1つのチケットは1つのissueのみ管理すべきだと思います。

また、大量のpull requestを処理する管理者を想定すると、やはり2つに分けた方が親切なように思います。結局、この記事で紹介している手順って中の人に少しでも楽させてあげようよ、っていう手順なんですよね(正確には、あるプロジェクトの中の人が楽をしたいから書かれたドキュメントですが)。中の人にcherry-pickさせたりrebase -iさせたりしても構わないという前提であれば、上のプロセスは複雑すぎると思います。

逆に、分かれている方が不便なことってあるんでしょうか。conflictする確率が高まったりすることを危惧されていたりしますか?

bleis-tiftbleis-tift 2011/06/10 20:58 > GitHubのissue tracking system上でも「修正Aと修正B」というようなチケットが立ってしまいます

GitHub の ITS は使ったことが無いのでこの部分はわからないですが、1つのチケットが1つのissueのみ管理すべき、というのは同意です。
ちょっとこの辺は調べてみる必要がありそうです。

> 結局、この記事で紹介している手順って中の人に少しでも楽させてあげようよ
> 逆に、分かれている方が不便なことってあるんでしょうか。conflictする確率が高まったりすることを危惧されていたりしますか?

ここが意見の異なる部分のようです。
単純作業 (それこそ確認した後は手作業が本来いらないはずの作業ですよね) を何回もやるより、注意を要する作業を一回だけやる方が実は楽なんじゃ?という。
で、不便な所はまさにそこです。何個も pull request が送られてきたとして、どれかを適用したことによってほかのどれかが conflict を起こしてしまう状況です。

hnwhnw 2011/06/14 17:25 お返事遅くなりました。conflictについては考え過ぎの部分もあるように思います。
(1)僕の2個のpull requestがお互いにconflictするパターン
(2)僕の2個のpull requestが他人のpull requestとconflictするパターン
のうち、(2)については僕が1個にまとめて投げようが2個に分けて投げようがconflictしますから、どっちでも構わない気がします。むしろ、conflictするカタマリの大きさが小さくなった方が解消が楽かもしれません。
一方で、(1)のパターンで2個に分けるのは確かに問題ですね。この場合は自分でconflictするのがわかるはずなので、1個にまとめてしまうのも手でしょうし、僕なら1個目がacceptされるまで2個目を隠し持っておくかもしれません。
ちょっと前のコメントでbleis-tiftさんが例として挙げられた、バグ修正+新機能追加のようなパターンであれば、バグ修正ブランチから新機能追加ブランチを生やして、それぞれをpull requestできれば良い気がしますが、そんなことが出来るのかどうかは把握できていません。これは宿題にさせてください。

また、pull requestを複数に分けてしまうと単純作業が増えるというのはGitの作業だけ見ればその通りかもしれませんが、GitHubの場合はITSも連動しているため、実際には両者の手間の差は大きくなるのかもしれません。次のURLを見ると、単にマージしてチケットをcloseするのであれば、GitHubのWebインターフェースから全て行えるようです。

https://github.com/blog/843-the-merge-button

トラックバック - http://d.hatena.ne.jp/hnw/20110528
 | 
ページビュー
1911471