初心者でもできるはじめてのPull Request
先日Google Summer of Code 2015に応募した。
応募プロジェクトはPython Software Foundation (以下PSF)のサブプロジェクト、SciPy。
ざっくり言うと「SciPyには使い勝手のいい微分モジュールが無いから実装しようぜ」という内容のproposal。
PSFは「proposal出す前に何かpull-req1本送ってね！」と言っている。
しかし僕はpull-reqなんて送ったこともない。（しかもこの時点で締切まであと半日だった…）
この記事は、そんな初心者こと僕がしどろもどろしながらpull-reqを投げ、mergeされるまでの記録である…。
（そういうわけなので、多少の無礼はお許し頂きたい）
大まかな流れ
- 解決できそうなIssueを探す
- forkしてbranchを切る
- コーディングする
- pull-reqを送る
- フィードバックを待つ
僕が半日でやったのはpull-reqを送るところまで。PSFは「とりあえずmergeされなくても送りさえすればOKよ」と仰っていたので、お言葉に甘えさせていただいた。
それでは僕の実体験を元にして、1つずつ見ていく。
1. 解決できそうなIssueを探す
GithubのIssueを見ると、日夜様々なバグや改善要望が書き込まれては解決されてゆく。
「大手OSSのSciPyともなると初心者が一瞬で直せるバグなんてあるのか…？」と絶望的になりながら眺めていたが、やはり物事は諦めてはいけない。
きっとどこかに誰でも直せるけどpriorityが高くないために直っていないバグが潜んでいるはずだ。
Issueを眺めること30分。僕が見つけたのはこのIssueだ。github.com
scipy.linalg.block_diagというブロック行列を生成する関数があるそうなのだが、Issueの内容を要約するとこんな感じだ。
scipy.linalg.block_diag([ [1, 0], [0, 1]], [])を呼んだら
[[1, 0], [0, 1]]を返すことが期待されるのに、実際には
[[1, 0], [0, 1], [0, 0]]が返ってきてしまうよ！なんで？
勿論実際にコードを読むまでは解決できるかわからないけれど、なんとなく解決できそうな雰囲気が漂ってる。
というわけでこのIssueを選ぶことにした。
当然自分で新たなIssueを立てても良いのだが、初心者には敷居が高いのでここでは既存のIssueを解決することを前提としている。
2. forkしてbranchを切る
「さあコーディングだ！」と行きたいところなのだが、コーディングする前段階の準備として何をすれば良いのだろうか？
pull-reqを送る場合には、一般的には以下の様なflowになるのだと思う。
- 本家リポジトリをforkする
- ローカルにcloneする
- 修正するバグ用にbranchを切る（絶対にmasterブランチで作業してはならない）
- コーディングする
- commitしてpushする
- Githubからpull-reqを送信する
- 待つ
originalは本家リポジトリ（ここではscipy/scipy）、yoursはforkした自分のリポジトリ
SciPyに関して言えばわかりやすくDevelopment Flowがまとめられているので、ぜひとも参照したい。
Development workflow — NumPy v1.9 Manual
これを熟読しておけば幸せになれるはず。
というわけでこの節ではまず上の3つについて説明する。
まずはforkする。forkはGithubの本家リポジトリページからできる。
forkしたらローカルの作業用ディレクトリにcloneする。
$ git clone https://github.com/[ユーザ名]/[リポジトリ名]
そして、作業用のfeatureブランチを作成する。
$ git checkout -b [新規ブランチ名]
新規ブランチ名をどうするかという問題があるが、僕はIssue#4596の修正ということで"bug#4596"というブランチ名にした。
（実はブランチ名の命名規則がどこかに書いてあるのかもしれないが、僕は見つけられなかった）
ちなみにcheckoutはbranchを切り替えるコマンドで、本来は存在しないブランチを指定すると怒られるが、bオプションをつけると新しいブランチを作成してそのままブランチを切り替えてくれる。
一応branchが切り替わったことを確認する。
$ git branch * [新規ブランチ] master
3. コーディングする
branchが作業用に切り替わっていることを確認した後、コーディングする。
これは本当に「自分でバグを検証して該当箇所を読んでバグを解決してください」としか言いようがない。
まあSciPyのようなライブラリであれば、モジュール化されているため読むべき箇所は絞られるので検証は比較的し易いのではないだろうか。
ちなみに、僕が取り組んだIssueの原因は、numpyのshapeに関する挙動である。
np.array([[]]).shape # => (1, 0)
空行列のshapeを取ると、1行0列と返ってくる。
あながち間違った挙動とも言えないのだが、block_diagはこの場合shapeが(0, 0)であることを期待していた。
そのため、空行列の場合に分岐処理を入れた（たった1行の修正だった）。
作業が終わったらcommitしておく。
ちなみにSciPyの場合はcommit messageの書き方も指定されているので、下記ページを参照すべし。
（OSSではよく行われるみたいだが、commit messageの頭にBUGやENH等のラベルをつける）
Development workflow — NumPy v1.9 Manual
4. pull-reqを送る
まずは自分のリモートリポジトリにpushする。
その前に、念のため作業中に他の人が行った更新を取り込んでおく。（pull-reqはfast-forwardでmergeできることが望ましい）
本家リポジトリから取り込めるよう、upstreamという名前で設定する。
$ git remote add upstream [本家リポジトリURL]
本家リポジトリはどうやらupstreamという名前にすることが慣例のようだ。
下記のページを参考にしてfetch(&merge)するべし。
更新に追従できたところで、もう一度git branchで念のためfeatureブランチなのか確認する。
Gitは間違えてcommitしたりpushすると取り返すのが非常に面倒になるので、ここで念入りに確認することに越したことはない。
ここでpushしたいところなのだが、pull-reqを送る前にcommit logを整理しておくことが望ましい。
pull-reqを受け付ける側も、いくら変更内容がコード的に正しくても無茶苦茶なcommit logで自分のリポジトリを汚したくないはずだ。（受付側でもcommit logは整理できるが、大手OSSになればなるほどpull-reqの数は膨大なので、いちいちそんなことはやってられない）
Communityにもよると思うのだが、SciPyでは「1つのpull-reqは1commit」と要請されているようだ。
【参考】
Development workflow — NumPy v1.9 Manual
ということで、git rebase（git squashといわれることもあるようだが）を使ってcommitを1つにまとめよう。
1commitしかしていない人は気にする必要はないが、今後のためにも理解しておくとよいと思う。
rebaseやらmergeやらよくわかっていない人（数日前の僕）は、下記の記事を一読することを強く推奨する。
とてもわかりやすいし、Gitの操作に対して自信を持てるようになるので、非常にオススメ。
基本的にはsquashという操作によって不要なcommitを潰し、ひとつのcommitにまとめる操作である。
rebaseのinteractiveモード（-iオプション）を使うのがわかりやすいと思う。
$ git rebase -i upstream/master
これを実行すると、エディタが立ち上がって自分の変更分のcommit logが見られる。
はじめの1行だけ残して、残りのcommitは行頭のpickをsquashに変更する。
すると最初の1commitに残りのsquashされたcommitの変更がまとめられる。
保存してエディタを閉じると、次にもう一度エディタが立ち上がってcommit messageを編集するように求められる。
これもひとつだけcommit messageを残して、残りのsquashした分のcommit messageは削除する。
これで変更を保存して終了すると、rebase終了である。
pushの準備が完了したので、featureブランチをpushする。
$ git push origin [featureブランチ名] -f
- fはforceオプションで、rebaseして書き換えたcommit logでリモートのcommit logを上書きする。
forceオプションは実は危険な操作なので、くれぐれも自分の作成したfeatureブランチ以外に対しては行わないこと。
featureブランチでは他人が作業していないことが保証されているが、あろうことかmasterブランチに対してforce pushでもしようものならば、自分のcommit logでリモートリポジトリが書き換えられてしまうので、他人が作業しようとすると同期が取れなくて大変なことになる。
force push時に指定するbranch名は絶対に確認しよう。
自分のリモートリポジトリにpushが完了したら、Githubを開いてpull-reqを送ろう。
サイドバーにPull Requestとあるので、ここをクリックしてあとは流れに沿って進めばよい。
5. フィードバックを待つ
pull-reqが無事に送信されたら、管理者からのフィードバックを気長に待とう。
僕の場合はcommitをsquashしていなかったり、コード中のコメントが不足していたり、リグレッションテストを書いていなかったりで、何回か修正を要請された。
はじめてのpull-req（たった1行の修正だけれど）だけあって、管理者に「Thank you!」と言われたときはなかなかの感動モノだった。
おまけ: SciPyのtestについて
Pythonの単体テストについては全くと言っていいほど知識を持ち合わせていないので頓珍漢なことをほざく可能性があるが、遠慮なく指摘していただきたい。
SciPyのリポジトリルートにruntest.pyというファイルがあり、これが単体テスト用のスクリプトになっている。
これに-tオプションをつけてテストスクリプトを指定すると、指定されたモジュールのテストを行うことができる。
僕が今回修正したのはscipy.linalg.special_matricesなので、
$ python runtest.py -t scipy/linalg/tests/test_special_matrices.py
でテストできる。
テストに伴って、自分でリグレッションテストをテストスクリプトに書き加える必要がある。
順番が前後したが、各モジュールに対しtests/test_hogehogeというテストスクリプトがあるので、そこに追記する。
見よう見まねでassertionを入れれば大丈夫。
