Scala の開発に学ぶコードレビュー体制とプロジェクト開発

コードレビューについて Oh, you `re no (fun _ → more) より引用

単に普段の開発で使っている VCS でそれを行なっていました。 つまり、コードの中にコメントの形でレビューを書き、それをコミットする。 そしてそこから派生する議論も全てコード上のコメントで行います。 (もちろん複雑な話になった場合は直接の議論を行い、合議の結果だけを記しておく、なども当然あるでしょう。)


これをみて、「GHCと同様、レビューをコメントに直に書くのってScalaの開発にも当てはまってるじゃん!」と感じたのと、色々と今までScalaの開発を観察してきて思うところがあったので、分かる範囲で現在のScala自体の開発がどうなっているのか書いてみます。*1


まず、このblogを書いている2012年8月現在以下の様なツールを利用してます


以前は SVN で Trak でした。 github に移行する以前は、自分自身がコミット履歴をあまり追ってないので、よくわかりません。githubに移行した後の話をしようかと思います。詳しい時期は忘れましたが、githubに完全移行したのは2011年の後半だったと思うので、以下に書くのはすべてここ一年くらいのわりと最近の話です。*2

githubに移行した後、色々と試行錯誤した結果、開発体制も微妙に変化があり、あくまで個人的に観察してきた結果ですが、3段階に分けることができると思います。

  1. githubに移行したばかりで、色々決まってない。みんなが直接masterにコミットしたりもしていた
  2. paulp さんがなんかすげー勢いで開発。 scala/scalapaulp さんが全部管理してmergeをする。他の人は fork したリポジトリ(たとえば odersky/scala ) で開発
  3. ぬこが登場。レビュー制度の確立などがされて今に至る


まず最初の時期は、なんだか試行錯誤で、internal の ML などでも色々議論がありつつ、たしかそれほど fork したり branch きったりの方針も定まってなくて、 gitやgithubの特徴が生かしきれずにいた感じです。

で、ある時期を境に、 paulp さんが中央のscala/scalaを一括管理するようになりました。最近はそうでもないですが、一日にいくつもくる pull request を捌きつつ、自分もすごく大量のコードを書いていて、 paulp さんいったい何人いるんだよ、いつ寝てるんだよ状態でした。Ruby知ってる人には、paulpさん = 中田さん といえばわかりやすいかと思います。*3
paulp さんのforkのリポジトリのこの青の割合

とか見てもらうとわかるとおもいます。一時期全部のmergeを担当していたことを差し引いても、odersky先生↓よりかなり青いです。




あと、*4githubのgraphも貼り付けておきます



その後、 macro の開発が始まり、そのあたりで前述した「コメントにレビュー書き込む」というのが散見されるようになります。というかすごく会話してたりします。たとえば、 v2.10.0-M6 の時点のソースコード中を '@M ' でgrep すると以下の様になります

60件くらいでてきますが、@Mというのがなにをあらわすかというと、Scala作者のMartin Odersky の Mです。

ほかにも、macroを担当していた xeno-by さんのコメントや paulp さんのものもあります。
https://gist.github.com/3350608

ところで、githubなら、ソースコードの行の横のところを押すと、そこにコメントが書けますよね?もちろんそれもかなり利用されてます。

じゃあなぜ、ソースコード内に直接書いたのか?という疑問があります。このあたりも完全に推測ですが、 macro や odersky 先生が8〜9割を実装した SIP15のvalue classSIP18 の場合、細かくpull requestするのではなく、かなり大量のコミットをまとめて一度に送っていた*5ことと

また、自分でコードを書いていて途中で自分のコードに自信が持てなくなった時にも自己レビューを書くことが推奨されます。 この時間違っても FIXME だけ、などという意味のないコメントは書くべきではありません。

とあるように、自分に対するレビューというか、メモ書き的なものはやはり直接ソースに書いてしまったほうがやりやすかったのだと思います。なお当たり前ですが、コード内だけではなく scala-internal の メーリングリスト でも大量に議論はされています。あと、もちろん「コード中のコメントに直接書く」というのは、本体のソースコードを純粋に読みたい場合は邪魔になったりデメリットもあるだろうし、Scalaの開発においても別にこの方法のみを特別推奨しているといった感じでは無さそうです。色々使い分けたりしてます。おそらく、マクロ開発など大幅にコンパイラのコア部分をいじるのにあたって、odersky先生や、xeno-byさんが、直接ソースコード上で会話したほうがやりやすい場面があったので、ある意味自然発生的にこうなったのではないかという気がします。*6


一方は macro の開発、もう一方は value class など、両方共コンパイラ内部のコア部分に関わっているため、ぶつかる可能性のあるにも関わらず、

  • それぞれが fork => pull request
  • ソースコードのコメント上での直接のレビュー(というか会話?)

で、わりとうまくいっていた印象があります。


それと、jenkins連携の話です。 @kmizuさんが、以前以下のようなtweetしてましたが

https://twitter.com/kmizu/status/194110333865037824
https://twitter.com/kmizu/status/194772950552018944

ScalaのIssue Trackerがgithubと連携してないように見えるので、scala-internalsに疑問投げてみた。見た感じ、現在Scalaでは JIRA GitHub Connector
https://plugins.atlassian.com/plugins/com.atlassian.jira.plugins.github 使ってないように見えるけど、これ使うべきじゃね?という提案。 #scalajp

そういえば、githubのコミットとIssue Tracker(JIRA)が連携してないことについて、疑問をscala-internalsに投げたら、確かにその点は正しくて、 https://groups.google.com/forum/?fromgroups#!topic/scala-internals/zrpooujW8_8  http://www.jetbrains.com/youtrack への移行などを検討してるとかいう話が。あと、2月時点でIssue Trackerについての議論が一度あったらしいということをAdriaanに教えてもらった。 https://groups.google.com/forum/#!topic/scala-internals/zOFLfJfq3ak/discussion #scalajp

それかきっかけがどうかわかりませんが、色々議論があったあと、現在は以下のようにjenkinsとgithubは連携しています*7 *8

  • pull requestがあると、それをjenkinsが検知して、requestに対するbuildを勝手に始める
  • なおかつ、"build始めたよ" って ぬこ がそのpull requestのコメント欄に勝手に書き込む
  • buildが終わったら、buildが成功したか、失敗したかを、また書き込む

また ぬこは賢いので PLS REBUILD ALL って書き込むと、それを解釈して、もう一回 build してくれる機能とかあります!!

最初githubに移行してしばらくはこの仕組みはありませんでした。



最後にレビューの制度ですが、pull requestやレビューのフローについてのdocumentがgithubwikiにあります。

https://github.com/scala/scala/wiki/Pull-Request-Policy

コード内のモジュールの種類(コンパイラ型推論の部分なのか、バックエンドのバイナリを吐く部分なのか、libraryなのか、ビルドのスクリプトについてなのか etc ... )によって、基本的な担当者とレビューする人が決まっています。
そして、(github移行当初はなかったのですが)現在では原則として、pull requestを送った人自身が誰にreviewしてもらうのかを指定し、必ず1人以上のreviewが通ったあとに本体にmergeされるフローになったようです。また、githubでは同じリポジトリ間でも、branchが異なればpull requestを送ることができるので、paulpさんなどが scala/scala で直接作業したとしても、テストを走らせるために自身で scala/scala の topic branch から master に対して pull request を送っていたりもしています。


このように、

  1. まずpull requestをトリガーに自動でテストが走る
  2. もしテストが失敗すれば、pull requestを送り直すなどする
  3. テストが成功した後に、レビューを始めれば良い( jenkinsのぬこが、build始まったことと、テスト成功したことを教えてくれるのでそのタイミングがわかりやすい)


というフローになっていて、

  1. pull request毎に対して必ずテストが走る
  2. 特定の人に負荷がかかり過ぎないように、事前にある程度大雑把に担当者を決めて分担しておき、誰かが必ずレビューした後にmergeする


という方針になっていることにより、bugが入りにくく、なおかつ分担して並行で効率良く開発できる体制が確立されているのではないかと思います。

現在Scala自体を全部を最初からbuildして全部テストする場合、1時間とか平気でかかるので、pull request毎のテストをjenkinsを使わずに手動でやるのはもう不可能でしょう。


OSSですべでgithub上でオープンに行なっているからこそ、色々できている面もあるとはおもいますが、こういう例を見本というか参考にして、実際の業務でも役に立てる面が多くあると思います。


とくに、gitやgithubの利用の仕方を見てると、うまく機能を使っていて、やはりとても便利で効率的だなぁ〜と実感します。


こんなのを観察しつつ、自分もかなり細かい微妙なところですが、最近いくつかpull request送って採用されたりしました。

@kmizu さんが以前

Githubを使ってScalaの開発に貢献しよう

というのを書いてるので、これを参考に皆さんも出来ることがあれば貢献しましょう。
また直接貢献しなくちょっと観察するだけでも、上記で書いたような「jenkinsやgitをどのように使って開発しているのか」をリアルタイムで知ることができ、その知識を活かせるかもしれないので、暇な時にgithubを覗いてみる程度でもオススメです。



あと最後に、他のOSS言語でのこういった開発体制がどうなっているのか気になったりするので、他の言語のversionを誰か書いてくれないかなぁーとちょっと思ったりしてます。Scalaでのgitやgithubでの開発を眺めてると、SVNでの開発とか想像がつかないとかあったり・・・

*1:中の人ではなく、あくまで一個人として外側から観察した結果なので、色々推測も入っています。事実関係で明らかに間違いなどがあれば教えてください。

*2: つまり2.9系は出た後なので主に現在行ってる2.10系の開発

*3: 元の開発者であるmatz=odersky 先生より、ダントツでコミット量が多いという意味

*4:最初に書いたときにはgithub側のエラーでみることができなかった

*5: pull req送る前の段階でgithubのコメント機能使うとしたら、 odersky/scala のほうにコメントしないといけなかったりして、コメントがそこら中に散らばってしまい、微妙だったのかなぁという予想

*6: さらに調べたところ、 @M のコメントはversion 2.6とか2.7とか数年前の古いソースコードにもありました。積極的に会話していたかどうかはべつにして、odersky先生が自分の名前つけてコメント書く習慣自体は昔からあったようです。

*7:jenkins自体の利用は、だいぶ前からしていた

*8: issue trackerとgithubの連携は未だに微妙?というか、そのあたりがどうなっているのかは自分が把握できてないので、詳しい言及は控えます。誰か知っていれば補足お願いします