肉とビールとパンケーキ by @sotarok

少し大人になった「肉とご飯と甘いもの」

「Pull Request」 はオープンソースに限らず使える優れた開発フローだ

Dev Git

チーム開発において、「チケット/Issue」「TDD」「コードレビュー」など、ソースコードの変更に対する効果的な開発フローについてよく考えるのだけど、なんにしてもこのあたりは非常に課題が多く、各社各コミュニティで色々なやり方が模索されているポイントだと思う。

で、まぁご多分に漏れず僕もよく考えるわけだけど、現状その過程で Pull Request こそが非常に効果的なのではないか、と思うので、ちょっとまとめてみようかと思う。
もちろん、言うまでもないようなことだよ、という人もいるかもしれないけど、そういう人がたくさんいると、非常に喜ばしいことだね。

Pull Request とは

GitHub でこう呼ばれているので、こう呼ぶことにするが、ここでは、複数のリポジトリ/ブランチ間でのオープンな patch のやりとりのことだと考える。
あと、自分が使っているのが Git なので、ここでは Git について扱うけど、Git 以外の分散VCSでも当てはまる議論だと思う。

前提条件
  • 複数人で1つのソフトウェアを開発する
  • 分散型VCS (Git。もちろん Mercurial などでも良いと思う) で、各自がリポジトリを保持している。
  • チーム開発なので全員が同じものを開発していて、リリースマネジメントの過程では、どこかに「マスター」的なものが存在する
    • つまり、ここからリリースしますよ、という大元。
    • Linux などでは Linus が持っているリポジトリ、なのかもしれないけど、大抵の場合はGitサーバ (e.g. GitHub) などのようなものが存在しているよね？
  • 各自は自分のリポジトリで、開発ブランチまたは開発ブランチから派生したブランチで開発を行う
Pull Request

フローを手順化すると、概ねこういうことになる:

  • あるリポジトリ A の develop ブランチから fork したリポジトリ A' の develop ブランチがあり、そこに変更を加えたコミットが存在する。
  • リポジトリ A の管理者 (あるいは、その変更箇所に対する責任者) に、「A' の develop ブランチに変更おいてあるので pull してくれ」と要求する
    • それが Issue (チケット) 化される
  • A の責任者がそれを確認し、A' -> A に merge を行う。


ちなみに、GitHub では、fork して変更して push し、元のリポジトリのページを開き、Pull Request というボタンを押して、どのブランチをどのブランチに Pull Requset をするか、という確認をし、確定すると完了。Request された人は、diff を確認したり、その変更されたブランチをチェックアウトしてテストを行ったりして、問題がなければ merge 、というフローとなる(fast-forward 可能であればウェブ上からもボタン1つで merge できる)。

ソースコードに対する責任とカオス化

複数人での開発で必ず伴うのは「人が書いたコードでは足りない機能を実現する」ことだ。

ここに登場するのは、ある機能を提供するサービス *1 と、それを書いた Alice さんがいるとする。

このとき、ソースコードに対する責任は、Alice さんにある (とする)。これは、よくあることだと思う。*2

そして、そのサービスを利用した機能を実装していた Bob さんが、このサービスで提供されている機能だけでは足りないことに気づいたとする。そうしたときに、いくつかのやり方があると思うけど、それをちょっと書きだしてみようかと思う。

変更・対応を依頼する

ひとつは、Alice さんに「こういう機能が足りないんだけど、追加してほしい」と依頼することだ。これは、縦割りになった会社や、そもそも会社が違ったり、クローズドなライブラリを利用している場合にはよくあることだと思う。

ただ、 オーバーヘッドが大きいという問題がある。Alice さんがいつ対応してくれるか、という問題 *3。変更の依頼・対応は、コミュニケーションコストが大きいし、「今すぐ欲しいのに」にはどうしても対応できない。さらに、コミュニケーションミスで、変更されたのに依頼された機能が提供されない場合もある。そしたら差し戻しだし、さらにコストはかさむ。

しかし、ある程度これは、業務形態とか色々な要因 (外部サービスを利用する場合など) によって、たとえオーバーヘッドが大きくても、こうせざるを得ない場合も多いね。まぁそれは仕方がないけど..

変更し、確認をとりコミットする


よくコミュニケーションのとれたチームではこれが出来ると思う。
Bob さんも、ある機能を追加したい場所を探し当て、そこに変更を加える。diff を見せて、「こういう変更をしたいんだけど」と Alice さんに確認をとる。Alice さんは、「大丈夫だ問題ない」と答え、Bob さんはそれをコミットする。

変更せずに解決する方法を探す


当該箇所には変更せずに自分の希望する機能が提供されるようにする。
これは、Aliceさんの責任範囲には影響しない自分の責任でコードを追加するというイメージ。

しかし、こういうことを繰り返すと、謎のオプションが増えたり、似た様なメソッドが増えたりして、後述のカオス化が完成する。

勝手に変更する



最も悪いのはこのパターン。勝手に変更してしまう。あとから Aliceさんが「おいちょっと何この変更」となることは目に見えている。
テストでこけてくれればまだ良いが、気づかずにバグが混入している場合もある。

カオス化

一番ツライのは、「変更せずに解決する方法を探す」と「勝手に変更する」だ。
Alice さんが作ったらしき機能に変更を加えずに自分の欲しい機能を追加していったり、あるいはAliceさんの意図とは別の意図がコードにどんどん追加されたりすると、そういったものが積み重なって、例えば「似た様な機能を提供するメソッドが別名でたくさんある、しかもいたるところで両方が使われているしどっちも消せない手が出せない」みたいなことがおこってしまいにはちょっと違うことをやる似た様な機能について更に同じようなことが繰り返される。カオスである。

こうなると誰も手を付けられないので「あーそこは樹海だからさわらないで」みたいなものが完成する。おめでとう。

Pull Request がもたらす「小さなパッチ」とコードレビューとリファクタの機会

そもそも、Git の利点として、自分の元にリポジトリを持てる、気軽にブランチを切り気軽にマージできる、というのがあるけど、Pull Request こそ、これを最大限生かした理想的な開発フローなのではないか。

上述のやりとりを Pull Request に置き換えると、次のようになる。

  • Alice さんが責任をもつ箇所について、Bob さんが変更し、patch をつくる (小さなパッチ)
  • Pull Request を送る
  • Alice さんがそれを確認する (レビューとリファクタ)
  • 変更を取り込む

上述でいうところの「変更し、確認をとりコミットする」によく似ているけど、

  • Git の分散性、ブランチ機能を生かした効率的なフローがシステムよって提供され、
  • オープンにやりとりされる、

という点が異なる。

小さなパッチを繰り返すこと

ソースコードへの変更は、大きければ大きいほど影響範囲やバグの混入する確率も大きくなるので、パッチは小さければ小さい方が良い。
できれば単一機能についてその都度パッチがあるのが良い。

だから、小さな変更でも、自分の責任下で済まないソースコードの変更は、すべて Pull Request になっているべきだ。小さな diff であればあるほど、小さなコストで確認ができる。それを繰り返すことが重要だ。

コードレビューの機会を設けること

コードレビュー会のようなものが開かれるチームもあると思うけど、色々な変更が含まれた大きな diff や、複数人が責任をもったソースコードについて、いっぺんにたくさんのコードをレビューするのは非常に大変だし、コストが大きい。その点、Pull Request であれば、小さな patch ごとにレビューが入るため、コストが小さく、議論が1点に集中できるため質も高い。

また、防げるべきことが防げる、という利点もある。
Bob さんは、その変更の影響範囲はわかっていたつもりで、それに対応したつもりだけど、想定できていなかった影響範囲について、Aliceさんのチェックが入る。これは非常に大きなことだ*4。「勝手に変更された」場合、あとからこの変更を知り「ちょっとなんでこういう変更になってるの？」となってしまうし、できるだけそれは避けたい。

コードに対するチェックの目は多ければ多いほうが良いが、とはいえ毎回全員の変更を誰かがあるいは全員がチェックし合うのは非常に大変。自分が書いたことのある単一機能に対して他の人が変更加えたいときに、どう変更されるかをチェックできる、という仕組みが Pull Request によって、コストの低い形で提供される。

リファクタされる

Bob さんが欲しい機能についての変更がレビューされるということは、Aliceさんが、それがその機能を提供するための変更にとって良い変更なのか、がチェックされることになる。
あるいは、「このメソッドはこういう意図で使われるからこの変更はココじゃないほうがいいなあ、こっちのメソッドにオプションを作ろう」のような提案ができるかもしれない。

小さなdiffでリファクタされることは、カオスになりきった大きなコードをあとからリファクタすることよりもコストが非常に小さく済む。

オープンであること

Alice さんと Bob さんが、こういうやり取りをしてこの変更が加えられたのだ、ということが他の人に分かる形で残る、というも非常に良い。

上記の「変更し、確認をとりコミットする」では、Alice さんと Bob さんの間では共通認識の確認がとれるが、そこに Charlie さんがまた変更したいと思った場合に、その時のやり取りがわかるかわからないかで、考えられることが違うと思う。その機能を提供するコードが、どういう意図で書かれているのか両人の意図がすぐにわかるはずだ。

まとめ

  • ちいさなパッチを常々レビューしよう
  • そのために、仕事でも Pull Request を使おう。
    • 「GitHub を仕事で使う」はモチロンOKだと思います。
      • お金のあるところは GitHub Enterprise が手っ取り早い。まじ高いです。
      • そうでないなら Organization で有料プランを使うのがよさげ
    • GitHub はちょっと... という人は Gitlab オススメ。Merge Request という機能があります。 *5


ちなみに、Redmine で Pull Request 的なことを何度かやってみたけど、チケット化して当人同士で管理するのはできるけど、どのブランチからどのブランチへどの diff を? のような記録を明示的に残すのが難しいなーと感じた。


繰り返しになるが、patch のやり取りやコードレビューを、システムによって効率的に低コストで行うことができるようになったことが、Git の、GitHub の革命であり、これを開発フローに取り込むというのは非常に有効な手段だと思う。


(追記) :

すいません、すいません、本当に忘却してました。
メールでのやりとりで、request-pull / am でパッチのやりとりをする機能が Git にはありますね。


オススメ:
入門Git (濱野 純(Junio C Hamano)) - Amazon

*1:とここでは呼ぶけど、MVCでいうところのモデル的な場所のこととか、リポジトリクラスとか、まぁだいたいそういうイメージ。

*2:ちなみに、ちょっと横道にそれるけど、自分が書いたコードに対する責任をちゃんと持つようにしましょう

*3:Aliceさんがすぐ対応してくれる場合もある。だけど、その場合Aliceさんにとっては差し込みの仕事だし、これはAliceさんにとってのコストが増大する

*4:テストが書いてある、というのも重要だけど、それは別の話になるので今回は割愛

*5:そういや横道にそれるけど、何度も Twitter などで言っているけど、GitHub クローンなどと称されるプロダクトはちょいちょいでてきてるのだけど、Pull Request こそが GitHub が起こした革命だと思うので、これが無いものは GitHub クローンなどではなく、ただの Git リポジトリビューワーですね。