Ruby
Rails
GitHub
初心者
OSS

RailsわからないマンがRailsにプルリク送ってみたけどマージされてない話

先日、Ruby on Rails にプルリクエストを出してみました!Rails歴が浅いながらも頑張って作ったのですが、結局マージはされていません。しかし、初めてやってみて興味深いことが多かったので、少しでも共有できたらと思います。

書いている人のスペック

  • 年齢: 19歳(プルリクエストを出した3月当時)
  • 職業: 大学生
  • プログラミング歴: 中3くらいからちょいちょいAndoridでアプリを作っていたものの、当時はクラスやインスタンスといった概念すらわからず、コピペでとりあえず動かしていた。昨年4月に大学に入学してから本格的にRubyとRailsの勉強をはじめ、6月に企業でインターンを始めて今に至る。

プルリクエストの概要

今回僕が出したプルリクエストはこちらです。
Add use_year_names option to date_select tag
要するに、Railsのdate_selectタグの年選択欄で西暦以外も表示したいというプルリクエストです。
インターンの仕事で、こんな感じの年選択欄を作ってくれと言われたことがありました。
スクリーンショット 2018-03-08 17.23.19.png
しかし、Railsのdate_selectにはこんな風に西暦以外の値を表示するオプションがありませんでした。
調べていると、実際に同じ問題に苦しんだ方の書いたQiitaも見つかりました。
Railsでdate_selectの和暦版入力フォームをつくる
僕はこれとは違う方法で実装しましたが、やはり苦労しました。その時、「このくらいRailsの標準機能にあってもいいのになあ・・・」という思いが芽生えました。実際、月の入力欄にはuse_month_namesというオプションが存在したので、その思いはなおさら強くなりました。
前々からいつかOSSに貢献してみたいという思いは持っていたので、これを機にRailsにプルリクエストを出してみようと思い、色々調べ始めました。

プルリクエストを出すまで

まず最初に断っておくと、僕がプルリクエストを出すまでの流れは基本的にRailsガイドのこのページ(Ruby on Rails に貢献する方法)内の 4 Railsのコードに貢献する の流れに沿っています。細かい具体的な話とかはいいから、とにかくRailsにプルリクエストを出してみたい!という方は、こちらを読まれることをオススメします。

開発するための環境を整える

早速開発を始めましょう。まずは環境構築です。Rails Guide では、Rails development boxを使う方法が勧められていますが、僕の手元ではうまく動作しなかったので、自力で環境構築を行いました。Rails コア開発環境の構築方法からもわかる通り、ここでセットアップするものはRails開発者にとっては馴染みの深いものばかりなので、特に支障はないと思います。自分のPCにgithubからrailsリポジトリをクローンしてきます。その後、開発を進めるために新しいブランチを切ります。

git clone git://github.com/rails/rails.git
cd rails
git checkout -b use_year_names

その後、必要なgemを全てインストールしましょう。後述しますが、Railsにプルリクエストを送るにはテストの作成が必要です。その際にGemfile内のgemが必要になります。

bundle install

ついでに、自分の手元にあるrailsを参照するRailsアプリケーションも作りましょう。おなじみのrails new コマンドをrailsリポジトリ内で--devオプション付きで実行することで、手元のrailsを参照するアプリケーションが作成されます。

cd rails
bundle exec rails new ~/my-test-app --dev

これで準備は完了です。早速、開発にとりかかりましょう。

機能を追加する

膨大なrailsリポジトリの中から自分の修正したい箇所を探し出し、修正を加えていきます。
今回、僕が最初に思いついた修正は、

  <%= date_select('user_birthday', '', start_year: 1998, end_year: 2000, use_year_names: ['1998(平成10)', '1999(平成11)', '2000(平成12)'])%>

のように、表示する年名を配列として列記してuse_year_namesオプションに渡す、というものでした。実際に、先述のuse_month_namesはこのように使われていたからです。
ではどこをいじればこれが実現するのか。まずgit grep date_select で関連していそうなファイルを探し出しました。幸い、date_select に関係していそうなファイルは下のファイルだけのようだったので、僕はこのファイルを読み始めました。
actionview/lib/action_view/helpers/date_helper.rb
しかしこのファイル、とにかく長く、さらに中でHTMLを生成するなどの見慣れない処理を行なっているためとにかく読みづらいです。それっぽいキーワードをひたすら辿って、お目当ての修正箇所を探し出しました。
どうやら、date_selectタグの年選択部分はDateTimeSelector#select_yearの戻り値として実装されているようです。

actionview/lib/action_view/helpers/date_helper.rb
def select_year
  # 中略
  build_options_and_select(:year, val, options)
end

このbuild_options_and_selectの中身を見てみましょう。

actionview/lib/action_view/helpers/date_helper.rb
def build_options_and_select(type, selected, options = {})
  build_select(type, build_options(selected, options))
end

この中では、build_selectで第一引数に合わせた<select>タグを生成し、その中身となる<option>タグはbuild_optionsで生成しています。
しかし、このbuild_options、 中身にできるのは基本的に "1" "30" "03"といった数字だけです。これでは"1998(平成10)"が入った<option>タグを作ることができません。
これに対処するべく、build_year_optionsというメソッドを作ります。

actionview/lib/action_view/helpers/date_helper.rb
def build_year_options(selected, options = {})
  year_names = options.delete(:use_year_names)
  return build_options(selected, options) if year_names.nil?
  start = options.delete(:start)
  stop = options.delete(:end)
  step = options.delete(:step)
  start.step(stop, step).with_index do |value, i|
    tag_options = { value: value }
    tag_options[:selected] = "selected" if selected == value
    text = year_names[i]
    select_options << content_tag("option".freeze, text, tag_options)
  end
  (select_options.join("\n") + "\n").html_safe
end

(かなり前に作った関数であり、後述の理由により記録が全て消えてしまっているので当時作られたものとやや異なる可能性があります)
このメソッドでは、date_selectタグの:use_year_namesオプションの値を最初に取ってきます。
そして、値がなかった場合は今までと同様の方法で<option>タグを作りますが、値があった場合はその値を使ってタグが生成されます。content_tag内で

<option value="1998">1998(平成10)</option>

のようなタグが生成され、それが連結されて返されるようなイメージを持っていただけたらと思います。

あとは、select_year内でこのメソッドを使うようにするだけです。

actionview/lib/action_view/helpers/date_helper.rb
def select_year
  # 中略
  build_select(type, build_year_options(selected, options))
end

さあ、これで最初に思い描いたようなセレクトボックスが作れるはずです。
せっかく前もって手元のrailsを参照するアプリケーションを作っていたので、これで動作確認をしてみましょう。適当なerbファイルに先ほどのdate_selectを書き込み、このアプリケーションに表示させてみます。

app/views/time/select.html.erb
<%= date_select('user_birthday', '', start_year: 1998, end_year: 2000, use_year_names: ['1998(平成10)', '1999(平成11)', '2000(平成12)'])%>

スクリーンショット 2018-03-08 17.23.19.png
ちゃんと表示されることが確認できました。

テストを書く

Railsでは、変更の一つ一つについてMinitestを使ったテストケースを書く必要があります。自分の変更したファイルに対応するテストケースのファイルが必ず存在するので、そのファイルにテストケースを追記しましょう。
僕の場合は以下のファイルでした。
actionview/test/template/date_helper_test.rb
僕はこれまでMinitestを触ったことはほぼ皆無だったのですが、幸い似たようなテストケースがファイル内にたくさんあったので、それを真似ることで問題なくテストを作成できました。

actionview/test/template/date_helper_test.rb
def test_select_year_with_custom_names
  year_names = ["Heisei 15", "Heisei 16", "Heisei 17"]
  expected = %(<select id="date_year" name="date[year]">\n).dup
  expected << %(<option value="2003">Heisei 15</option>\n<option value="2004">Heisei 16</option>\n<option value="2005">Heisei 17</option>\n)
  expected << "</select>\n"

  assert_dom_equal expected, select_year(nil, start_year: 2003, end_year: 2005, use_year_names: year_names)
  end

expectedという変数の中に、今回生成されてほしいHTMLを文字列として入れて、select_yearで生成されるHTMLと等しいかどうかを比較しています。
それでは、テストを実行して失敗しないことを確認します。railsの中には大量のテストケースがあるため、全部実行するにはものすごい時間がかかります。ここでは、修正したファイルはactionview/test/template以下にあるので、この下にあるテストを実行すれば十分でしょう。

cd actionview/test/template
bundle exec rake test

#中略

Finished in 4.219940s, 40.2849 runs/s, 71.3280 assertions/s.
170 runs, 301 assertions, 0 failures, 0 errors, 0 skips

一つのテストも失敗していないことがわかります。
これでコードを書く作業はほぼ終わりです。

変更内容をまとめる

railsの中には、様々なディレクトリ中にCHANGELOG.mdというファイルがあります。この中に、railsに変更を加えた一人一人が変更を書き残しています。自分の変更内容を短くまとめて追加しましょう。もちろん英語で書くことになりますが、変更が良ければ後々ネイティブの方々が英語は直してくれると思うので、そこまで心配しなくても大丈夫だと思います。
僕の場合、actionview/CHANGELOG.mdを編集しました。

actionview/CHANGELOG.md
*   Add `year_format` option to date_select tag. This option makes it possible to customize year
    names. Lambda should be passed to use this option. Example:

        date_select('user_birthday', '', start_year: 1998, end_year: 2000, use_year_names: ['Heisei 10', 'Heisei 11', 'Heisei 12'])

    The HTML produced: 

        <select id="user_birthday__1i" name="user_birthday[(1i)]">
        <option value="1998">Heisei 10</option>
        <option value="1999">Heisei 11</option>
        <option value="2000">Heisei 12</option>
        </select>
        /* The rest is omitted */

    *〇〇〇〇(僕の名前)*

これでファイルの編集は終わりです。いよいよ変更をコミットし、プルリクエストを出してしまいましょう。

コミット

いよいよ変更内容をコミットします。

git commit -a

でコミットメッセージを記入するためのエディタが開きます。Railsガイドでは、コミット内容が一目でわかるようなコミットメッセージを求められているので、できるだけわかりやすく書きましょう。僕の場合、先ほどのCANGELOG.mdと同様の内容を記述しました。

作業が長引いた場合は、この間にmasterに変更が加えられている可能性があります。最新のmasterをGithubから取り込んで、作業をしているブランチに統合しましょう。

git checkout master
git pull --rebase
git checkout use_year_names
git rebase master

git pullrebaseオプションやgit rebase自体については、この記事がわかりやすかったので参考にしてください。
git pull と git pull –rebase の違いって?図を交えて説明します!
大事なことは、マージコミットをrailsのコミットログに残さないということです。(普通にしていたら手元のとリモートのmasterがコンフリクトすることはないと思うので、なぜrebaseオプションをつけなければならないのか僕はよくわかっていません・・・)

プルリクエスト

さあ、ここまで来たらあとは変更をプルリクエストにするだけです。まず、Githubのrailsリポジトリ上でforkボタンを押し、自分のアカウント内にrailsリポジトリを複製して来ます。
スクリーンショット 2018-06-09 23.46.48.png
実際のプルリクエストは、この複製された<自分のユーザー名>/railsリポジトリから、rails/railsリポジトリに対して出されます。
まずは<自分のユーザー名>/railsの方に、今回の変更をpushします。

git remote add mine git@github.com:<自分のユーザー名>/rails.git
git push mine use_year_names

ここまで来たらあと少しです。New Pull Requestボタンを押して、rails/railsの masterブランチにプルリクエストを出します。

スクリーンショット 2018-06-10 0.00.33.png
プルリクエストにはわかりやすいタイトルと変更内容を記述しましょう。(僕はここでもCHANGELOG.mdを流用しました。)

ここまで来るとようやく、

スクリーンショット 2018-06-10 0.13.56.png
プルリクエストが作成されました!これで自分の加えようとしている変更がrailsを見にくる全員に公開されます。感慨深いものがありますね。

プルリクエストを出してから

フィードバックを受け取る

しかし、ここで油断していてはいけません。ここから、railsに携わる様々な方々からのフィードバックが待っています。彼らにとってrailsに取り込むに値するプルリクエストでなければ、masterにマージされることはありません。

初めは僕のような者が書いたコードが強いエンジニアの方々に叩かれるのではないかと想像していましたが、以外にもそんなことはなく、皆さんこの機能追加をどのように改善すればよいかを真剣に議論してくださいました。
僕が最も指摘された点は、「同じような表現の続く配列をオプション引数にとるような作りは美しくない」ということでした。僕もその点には完全に同意していたのですが、代わりの記法を思い浮かばずにいました。
そんな中、レビューしてくださった方の一人がこんな提案をしてくれました。

Actually, one good way that we can achieve this is maybe letting user pass in a proc?
date_select('user_birthday', '', start_year: 1998, end_year: 2000, labels_for_year_options: ->{ |year| convert_year_to_wareki(year) })

恥ずかしながら、僕は今までRubyのProcオブジェクトの存在を知りませんでした。Procというのはめちゃくちゃ平たくいうと関数を閉じ込めておくオブジェクトです。下のリンクの説明が詳しいかと思います。
Procを制する者がRubyを制す(嘘)
例えばconvert_year_to_warekiという西暦の年を受け取り和暦に変換するような関数をオプションに渡せば、先述のような<option>が生成されるようにしてはどうか、という提案でした。
これは明らかに僕が思いついたものよりも綺麗な実装です。急いで採用することにして、手元で作業を進めました。

コードを修正する

実はこの他にも、僕は何回かの指摘を受けました。それらを何回かのコミットに分けて修正した、最終形態がこちらです。

actionview/lib/action_view/helpers/date_helper.rb
def select_year
  # 中略
  build_select(type, build_year_options(selected, options))
end

def year_name(number)
  if year_format_lambda = @options[:year_format]
    year_format_lambda.call(number)
  else
    number
  end
end

def build_year_options(selected, options = {})
  start = options.delete(:start)
  stop = options.delete(:end)
  step = options.delete(:step)

  select_options = []
  start.step(stop, step) do |value|
    tag_options = { value: value }
    tag_options[:selected] = "selected" if selected == value
    text = year_name(value)
    select_options << content_tag("option".freeze, text, tag_options)
  end

  (select_options.join("\n") + "\n").html_safe
end

year_formatオプションが渡されていたらそれを使って数字を修飾し、オプションがなければ数字をそのまま返すyear_nameメソッドを定義することで、オプションがある時もない時も同じ動線で<option>を生成することができます。さらに、`date_selectを使う時も、

app/views/time/select.html.erb
<%= date_select('user_birthday', '', start_year: 1998, end_year: 2000, year_format: ->year { "#{year}(平成#{year - 1988})" })%>

と、以前に比べて簡潔にかけるようになりました。初期のバージョンと比べると格段に使いやすく、綺麗な実装になったと思います。

その後

変更をコミットしたら、基本的にそのままpushしてさらなるレビューを待ちます。しかし、railsのコミットログに余計なものが入っていてはいけません。マージされる直前には、git rebase -iしてコミットを一つにまとめます。
コミットのまとめ方に関しては、下のリンクが参考になります。
git rebaseでsquashする
ちなみに、僕は早とちりしてこの作業を早くにやりすぎたので、途中に書いたコードが全部消えてしまいました。自分のコードの変遷を後から見たかったので残念です。個人的には、コミットをまとめる前に一旦git checkoutして自分の手元に修正の履歴を残しておくことをオススメします。
また、長いことレビュー・マージを待っていると他の人の修正によって自分のブランチと masterがコンフリクトを起こすことがあります。そんな時は、この記事の「コミット」と同じ手順でmasterの変更を自分のブランチに取り込んでください。
ちなみに、僕がプルリクエストを出してから3ヶ月が経ちますが、未だにマージはされていません。レビューで指摘された修正は全て行ったのですが、railsリポジトリへのマージ権限がある方に気づいてもらえていないのかもしれません。それでも僕は、その日が来るまでたまにコンフリクトが生じたのを確認してはgit rebase -iし続けようと思います。

 

最後に

結局、僕がRails Contributorとなることはありませんでした(今の所は)。しかし、

  • Rails内のコードを読んだり、プルリクエストのレビューを受けたりすることで、今まで知らなかったRubyの機能を知れたこと
  • 新参者を迎え入れるOSSのコミュニティの温かさに触れたこと
  • 「コミットログを綺麗に」という哲学のもと、今まで使ってこなかったようなGitの操作を学んだこと など、僕がこのプルリクエストを作る中で得たものははかりしれません。きっとこの先長いエンジニアとしての人生に、必ず役立って来ると思います。もしもなんとなく怖い・大変そうという印象でこうした巨大OSSへのプルリクエストを躊躇している人がいたら、ぜひ思い切って一歩を踏み出して見てください。きっと想像よりもずっと大きなものが返って来ると思いますよ。