目次
今回は、松田明さんにお話をうかがう機会をいただきました。 松田さんと言えば、RubyとRuby on Railsのコミッタであり、世界で一番活発に活動している地域Rubyコミュニティの一つである「Asakusa.rbの創設者」でもあり、さらに世界最大のRubyカンファレンスである「RubyKaigiのチーフオーガナイザ」で、加えてKaminariやActionArgsなどを開発した「Gem作家」でもある、と活躍の幅が広くて紹介しきれません。
松田さんにインタビューした記事は、すでに世の中にいくつかあるのですが、今回はあまりお話しされているのを目にしたことがない「コードレビュー」や「レビュー自動化」を中心に、お聞きしていきたいと思っています。
CRubyでのコードレビュー
ー先日SideCIのBlogで、まつもとゆきひろさんのインタビューを記事にしたものがありましたが、その中で、まつもとさんにコードレビューについてお話を聞きました。松田さんから見たときに、補足やあるいは異論なんかはありますか？
この話は面白いですね。仕事でもオープンソースでも「コミットしたコードを自分が将来的にメンテナンスすることになるけど、それをできるのか」というのが基準になっているとのことですが、これは、すごくちゃんと言語化されてて凄いなと思いました。その通りですよね。「コードを引き受ける決心」という短い表現もありましたが、なるほど、と。言語化が上手い人だなと改めて思いました。
ここで、まつもとさんがレビューについて話しているのは、これは恐らく彼が最近やっているmrubyにおける話ですね。これはいわゆるCRubyとはちょっと違います。
CRubyプロジェクトにおいては、そもそもコードレビューという概念がなくて、コミッターは書いたコードをSubversionのtrunkに直接コミットしています。Pull Requestを出して事前にCIを回して、みたいな感じでコミットするまでの間に誰かの目を通す仕組みというかプロセス自体がないんですよ。
そうしてtrunkにどんどんコードがコミットされるわけですが、そこでRubyCIというものがあって、定期的にtrunkから最新のコードをチェックアウトして、Rubyがサポートしている環境でテストを実行して、結果を表示します。マトリクスがすごくいっぱいあって、見ている人はこれを見ていると。だいたいそんな感じで回ってます。
- RubyCI https://rubyci.org
それから、CRubyで一番レビューに近い何かがあるとしたら、それはPB memoという、コミッターのnagachikaさんが毎日欠かさず更新されているブログですね。nagachikaさんは全てのコミットを監視しているので、何かが壊れたらここに書かれて、それで他の人も気付いて次の日に修正される、みたいな。
- PB memo http://d.hatena.ne.jp/nagachika/
ーいきなりtrunkが壊れると聞くと、けっこう乱暴な話にも聞こえますが…
でもtrunkだから壊れててもいいんじゃないですかね。リリースまでにはだいたいちゃんと直るので。
ーそれに対してmrubyはGitHubでということですか。
そうですね。
CRubyに関してはこういった感じで、特に誰もレビューもしてないし、すごく頻繁に壊れているというのが、おそらく反省点としてあって、まつもとさんが次のRuby処理系を作るプロジェクトを始めた時に、GitHubでやってみようってことにしたんだと思うんですよね。
mrubyでは、直接masterにpushされるのではなく、Pull Requstを作ってCIを回してレビューをしてからマージされるといういまどきっぽいプロセスですね。これは、まつもとさん1人の手に負えるサイズでやっているというのもあると思うんですが、CRubyの今の開発体制に対する、まつもとさんなりの反省というか改善も入っているんじゃないかと思ってます。
コードスタイルと規約
CRubyのコードスタイル
ーコードのスタイルをどうしていくか、みたいな問題は、Rubyの開発ではどうなっているんでしょうか？
CRubyのコードに関しては、コードスタイルの統一みたいな問題は、さっきのまつもとさんのインタビューで言われてたとおり「元々のファイルの雰囲気を読んで、各自好きなスタイルでいい感じになるようにコミットしてね」という感じです。コーディング規約とかは特にありません。すると実際にどうなっているかというと、たとえばよく話題になるのは、1つのファイル内にタブとスペースが混在してたりします。適当なCのファイルを開くとすぐわかると思うんですが、インデントがぐちゃぐちゃに入り乱れていて、なんかリアス式海岸みたいになってます。
リアス式海岸。Photo by Serkan Turk on Unsplash
ー私もcompile.cとかを読んだときに困りました。8タブ表示にするとなんとか読めるようになる。
標準的なタブ幅2の設定で表示したときのcompile.cの抜粋。whileよりもifが左に表示されるため、コードの構造と一致しておらず、理解を妨げる。
タブ幅8で表示すると、while内のifの位置がコードの構造と一致するようになる。
僕の手元のエディターだと2タブなので、凄くキザギザになっちゃいますね。そこで、mrknさんが作ったRuby開発者必携のVimプラグインがあって、これを入れておくとそこらへんをうまいこと吸収してくれるようになってます。
https://github.com/mrkn/vim-cruby
まぁ、ソースコードを読み書きするために、こういう努力が必要になっちゃうわけです。
ーちなみに、コミッターの皆さんの間ではタブとスペースのどっちが優勢みたいな感じはあるんですか？
人数はスペース派の方が多いはずです。でもパッチモンスターの中田さんがどちらかというとタブ派なので手数が多いという。
笹田さんはタブですね。中田さんはどっちでもいいと言ってるんですが、最近はタブにしているようです。そしてスペース派の急先鋒は、僕が把握してる限りakrさんなんですかね。akrさんがコツコツと地道に、自分がいじった行はタブをスペースに書き換えてコミットするみたいな活動をやっておられたはずです。そしてまた別の人が書き換えた時にはスペースがタブに置き換えられたりする、タブスペース合戦みたいなことが繰り広げられていたりします。そこに3タブ派が入るとさらにカオス状態になるかもしれないですね。幸いCRubyには3タブ派はいないと思いますが。
このタブスペースの話が一例ですけど、コードスタイルをみんなで足並み揃えてなにかにしようというのはとにかく一切ないです。
その結果、GitHubのレポジトリを作ってプルリクエストを受け付けるようになってみると、コントリビューターの皆さんからどっちに揃えていいかわからないという声は当然あるわけで。昔はそういう時代じゃなかったけど、今のGitHub時代では「最低限のコーディング規約はあったほうがいいんですかねぇ」とは思います。
そして、それをやっているのがRuby on Railsプロジェクトですね。
Railsのコードスタイル
ーRailsプロジェクトでは、コーディング規約があってRuboCopで検査しているわけですね。
はい。本当に最低限の所だけ機械にやらせるみたいな感じで。RuboCopデフォルトのルールは無視してしまって、自分たちで「最低限ここだけは」というルールを入れています。
- .rubocop.yml https://github.com/rails/rails/blob/a77447f4da15dd3c2b02563fa6f5769c1d91e1bd/.rubocop.yml
ー最低限のルールというのはどんな感じですか？
例えば、有名なDHH Indentationがあります。privateのときにそこから下のコードを全部右にずらすっていうキモいやつ。 IndentationConsistency = rails これは名前が rails になっちゃってることからわかるように、Rails以外では見たことないですね。プロジェクトオーナーが頑なに主張するので、今はこれが採用されています。
あと僕が最高に気に入らないのが StringLiterals = double_quotes。つまり「文字列のクォート記号は全部ダブルクォート」。これはDHHじゃなくて誰かが雑に入れちゃったんですよ。既存のコードを片っ端から書き換えてくれちゃって。こんなどうでもいい理由でそういうことをされるとgit blame が汚れるんでほんと困るんですよねぇ。
あとは RequireParentheses = true も気に入らないです。メソッド呼び出しはカッコをつけてください。これは要らないと思うんだけどなぁ、Javaじゃないんだし。
あ、Railsでは SpaceInsideHashLiteralBraces = true で、Hashのリテラルの中にスペースを空けろなんですよ。これはマイノリティーだと思うんだけどなぁ。なんかHashってキュっと書きたいじゃないですか。なので、僕はスカスカして気持ち悪いなぁ、って思いながら書いてます。
要は、自分が仕切ってるプロジェクトだったら自分のスタイルで書きますけど、他人が書いてるコードのスタイルがちょっとぐらい自分と違ってもあまりに気にならないし、さらに言うとリポジトリーオーナーから「これでよろしく」というルールが提示されてるんであれば、僕は少なくともそれに従いますね。
Pull Requestを出す場合でも一緒です。既存のコードをじっくり読んで、なるべくそこのコードの文脈を読み取って、「きっとここはこういう風なルールでやってるんだろうな」というのを推測してそれに合わせたコミットをする、というのを心がけてるんです。そこは空気を読むっていうか。それもまたコードを読むときの楽しみの1つだと思っているし、そんなふうにして違和感の無いコミットを作る技術もOSSディベロッパーとしての大事なスキルのうちの1つだと思っています。
ー意外とたくさんルールがあるんですね。RuboCop導入の経緯とかルールの決め方とかはどんな感じですか？
やっぱりPull Requestのレビューの所で、「ここにTrailingWhitespaceが入ってます」みたいな不毛なやりとりは人間がやらなくてもいいようにしてる、っていうのが最初だったはずです。
どういうルールを採用するかというのは、チャットとかで誰かが提案して、「いいんじゃない」ってなれば入るとかそれぐらいですね。今見ると、いつの間にか結構増えてますね。最初は2、3個だったんですけどね。
一番最初のコミットを見ると面白いかもしれないですね。
だいぶ今よりシンプルですよね。この頃から括弧は必要とか、Style/Hash、Style/TrailingWhitespace、あとは2スペースノータブ。
最初はこれくらいから始まって、僕もこれくらいなら悪くないと思ってたんですけど、結構いつのまにか増えてますね。正直めんどくさいです。
これはやり方としては凄く参考になる部分がある気がしますね。実際に仕事のプロジェクトなんかに導入しようとしたときには、最低限「これはみんな許容できるよね」ってところから始めて、少しずつルールを追加していくという。RuboCopみたいなものに抵抗感ある人も多いので、いつのまにかRuboCopをじわじわと浸透させるには良いプラクティスじゃないでしょうか。
標準コーディング規約
ーGitHubとかクックパッドとか、コーディング規約を決めていて、公開までしている企業もありますね。
それを好む好まざるはともかく、そういう部分が決まってる会社は業務としては効率よくやろうと工夫して取り組んでる現場なんだろうなという印象はありますね。規約が公開されていると、もしかしたらそこに入社しようか考えている人が、「この規約は絶対に従えないから」と思って、最初から合わない企業に当たらないで済む可能性があるかもしれないですね。
ーコーディング規約があるおかげで効率が上がる感じはありますか？
正直、他人のコードが自分のスタイルとちょっと違っても気にしない社員ばっかりだと、コーディング規約がなくてもそれはそれで回ると思うんですけど。コードレビューでそういうのを指摘して仕事した気になっちゃうみたいな人がいるじゃないですか。そこを防ぐ効果はありそうだから、確かに価値はあるかもしれません。
「Hashのブレースの中はスペースを空けてください」とか、僕はどっちでもいいじゃん動くじゃんとか思うんだけど、そのやりとりをする時間を省略できるということですね。
1つ思うのは、いわゆる標準コーディング規約みたいなのがコミュニティー全体に、あった方が幸せなのかなあと。RuboCopのアレではなくて、ちゃんとRubyっぽいやつ。
昔は前田修吾さんのNaClコーディング規約がありましたね。あれは今となっては……という感じで、古すぎる感じがしますけど。80文字ルールを厳格に、とか、ないわーと思います。あとは青木さんのもありましたよね。改行をいっぱい空けろとかそこが印象に残ってます。すきま重要って言ってるんですよね。これは読み物として抜群に面白いですよ、すごく味があって。
ー標準の規約と言えば、Elixirは次のバージョンで標準コードフォーマッタまで提供することにしたと聞いたんですが。
最近の実装で、José本人ではなくて誰か他の人の提案だそうです。Goみたいなノリですよね。言語本体に標準コードフォーマッターが入るのは。
ー言語本体に入れるんなら、言語自体を変えてしまえば良いと思うんですが。
それをやると大きな非互換になっちゃうので。自由ではあるんだけど、標準のフォーマットは提供する。
Joséに聞いた話では、「そんなのはElixirらしくない」と最初は思っていたんだけど、提案者があまりにも猛烈にプッシュするので、「じゃあ」と入れてみたと。入れてみたら一発で気に入ってしまって、「最初からそうしとけばよかったと今は思ってるよ」って言ってました。
「そんなのを他人に強制されるのは気に食わない、ってRubyistは思ってるかもしれないけど、強制されるてみると楽になっていいよ」というふうにJoséは言ってましたよ。まぁ、Rubyが標準フォーマットを取り入れるとかはないとは思いますけど、1つ興味深い話ではありますよね。
ー松田さん自身のプロジェクトでは、RuboCopは使ってないんですか？デフォルトのルールはともかくとして、Pull Requestのコードを機械的に検査したいみたいなことはあるんじゃないかと思いますが。
RuboCopは自分のOSSプロジェクトでは使ってないですね。今のところ、スタイルに関する議論が面倒だと感じる規模でやってないから、その必要性を感じてないだけかもしれません。
Pull Requestをもらった時に対処する方法っていくつかあって、たとえばRailsみたいにレビューコメントで最高のPull Requestになるまでじっくり育ててからマージするという方法もあるんですが、僕はそのやりとりを省略しちゃって一旦masterに入れちゃってから自分で好みのコードに書き直す、ということが割とありますね。
たとえばKaminariくらいの規模だと、とりあえず、はいはいってマージしちゃうんですよ。それで後でコードをいじって書き換えるわけです。でもそれは、スタイルよりはロジックとかの部分が多いですかね。
あ、でも同じ事を須藤さんにやられた時は、「だったらPull Requestのときに言ってよ」と思ったので、もしかしたら自分ではやるけど他人にはやられたくないのかもしれないです。ワガママですね。
これがそのPull Requestなんですけど、 $LOADED_FEATURES の文字列を書き換えるとかいうありえない感じのsuper ugly hackみたいなやつを、「こうしないと動かないんですよ」っていう説明をしつつ「本当、汚いコードでごめんなさい」って言ってPull Requestで出しました。そしたら「It’s surprising solution」っていうお褒めのお言葉をいただいて、それでマージしてもらった後に須藤さんの好みのコードに書き換わっていましたっていう。
コードレビュー自動化とSideCI
僕の経験では一回、すごい細かい .rubocop.yml を採用してみたっていうお仕事のプロジェクトがありました。それはそれで .rubocop.yml の内容を決める作業にすごい時間を取られて、生産性下がってるじゃん、ってことになって、結局何もない状態に戻したということがあります。
やっぱり DisabledByDefault ですよ。そうじゃないと、RuboCopのバージョンが上がると、まず bundle update が通らないんですよね。毎度毎度新しいルールが増えすぎて。
これではプロダクトのデイリーの bundle update の妨げになるわけで、良くない気がしますね。開発生産性を上げるために入れてるのに、むしろ足を引っ張ってるっていう。
ーその問題は、SideCIを使うと改善するかもしれませんね。 それはCIでRuboCopを実行しているから起きる問題で、CIから切り離してSideCIにRuboCopを任せればそういうことは起きません。SideCIでは、Pull Requestで変更された部分だけが対象になるので。
そうすると、自分のせいじゃない変更で、いつか誰かが埋めてくれた地雷を後から踏む可能性があるということですよね。まあ、それは git blame の犯人になるはずの人が責任を持って誰にも怒られないコードをコミットをすればいいということか。「これ壊したの俺じゃないのに」とかブツブツ言いながら。
ーもしくは、そもそも直さないか。SideCIはRuboCopが検査して出てきた警告を、もう一度人間がレビューして受け入れるかどうか決めるという風になっています。RuboCopと意見が合わない時は、とりあえず無視して先に進むってことができるようになってるんですよ。
それは技術的に言うと、RuboCopをignoreするコメントを自動で挿入してくれるということですか？それともSideCI側でデータベースを持っている？
ーデータベースをSideCI側で持ってます。コミットをまたいで見るので、「この無視された警告はこの行に移動して」みたいなこともトラッキングしています。 これで、RuboCopのちょっと怪しいルールなんかもガンガン有効にできるようになります。RuboCopと意見が合わないこともありますが、役に立つルールもあるので。
そうですね。確かに DisabledByDefault の場合は、例えばセキュリティ関連とか役に立つルールが入ってくれた時には気づけないってのは問題としてはありますね。
いや、さすがにちゃんと色々と考えて作り込まれてるんだなと理解しました。
RubyKaigi 2018
ー今年のRubyKaigiには、RuboCopの作者がスピーカとして参加するそうですね。RuboCopの話をしてくれるとプログラムにあります。
はい。bbatsovが今回初来日で、RubyKaigi初日のスピーカーとなっています。これはRuby 3みたいな話とは別の、今年のRubyKaigiの1つの注目ポイントですね。コーディングスタイルの話とかRuboCopがどうだ、とかいう話はRubyコミッターの間でも話題になることは多いんで、RuboCop作者本人がやってきて日本のコミッターたちと意見交換ができるというのはなかなか有意義なんじゃないでしょうか。
彼は去年、EuRoKoというヨーロッパのカンファレンスでキーノートをしていて、彼なりに将来のRubyのデザインについて話をしているんですが、ちょっと正直言ってRubyの良さを何もわかってない感じだったので、そのへんについて直接コミッターたちと話ができるのは彼にとってもいい機会になるはずです。
EuRuKo 2017 https://euruko2017.org Ruby4: To infinity and Beyond https://speakerdeck.com/bbatsov/ruby-4-to-infinity-and-beyond
ちなみに、彼の今回のプロポーザルには「話したいことがいっぱいありすぎて、40分の枠で何を話したら良いか迷っている」というふうに書いてくれていたので、僕の方からフィードバックとして「RubyKaigiは開発者のための技術的なことにフォーカスしたカンファレンスだから、テクニカルな話にしてくれ」という要望を伝えました。ので、今回はもうちょっと実装寄りの、面白い話が聞けるんじゃないでしょうか。
RubyKaigiには、SideCIからは私 (@soutaro) と @pocke が発表します。既に開催の直前ですので、なかなかRubyKaigiに参加しようというきっかけにはならないと思いますが、国内からならまだ間に合うはずですので、ぜひ皆さんも参加をご検討ください！