Symfony
SymfonyDay 11

SymfonyにPull Requestしてみました

先日、たまたまSymfonyにPull Requestする機会がありました。無事にマージしていただき、みつけた不具合も修正されたようなので、そのときの体験談をまとめようと思います。

不具合をみつけたきっかけ

EC-CUBEで利用しているSymfony Componentを、2.7.37から2.7.38に更新しようとしていたところ、EC-CUBEのTravis-CIのテストが通らなくなりました。

Formに対してテストデータをsubmitしている箇所で、こんなエラーが発生していました。

ContextErrorException in FileType.php line 43:
Warning: Invalid argument supplied for foreach()

EC-CUBEのテストコードが間違っているのか?と思い、最小限のコードを試してみたところ、やはり同じエラーが発生します。

$builder = $app['form.factory']->createBuilder();
$builder->add('file', FileType::class, [
   'multiple' => true,
]);

$form->submit([]);

FileTypeのコードを確認してみると、$event->getData()をそのままforeachループで回していて、getDataがnullを返した時にこのエラーが発生することがわかりました。

foreach ($event->getData() as $file) {
    if ($requestHandler->isFileUpload($file)) {
        $data[] = $file;
    }
}

Issueの投稿

とりあえずIssueに投稿しようと、Contributing to Symfonyに目を通しました。ちゃんとガイドラインが整備されているのはありがたいですね。

不具合報告の場合は、Reporting a Bugに説明があります。
書かれているのは基本的なことで、

  • ライブラリの使い方間違ってない再度ドキュメントを確認する
  • 不具合かどうか判断できないときは、SlackやStackOrverFlowで聞いてみる

といった、Issueを立てる前の注意事項であったり、

  • 再現手順や再現コードを載せる
  • 環境や利用しているバージョンなど、できるだけ詳しく説明する

といった、不具合報告についての注意事項が書いてあります。

実際にIssueを立てるときには、テンプレートが表示されるので、それに従って埋めていくだけです。他の人のIssueを参考に書いてみたのが以下です。

https://github.com/symfony/symfony/issues/25063

さきほど調べた、発生している事象と、再現コード、原因箇所を記載しています。

ちなみに英語はGoogle翻訳です!

Pull Requestを送ってみる

Issueを立てたらすぐに反応をいただきました。

Hi @chihiro-adachi I think this should be handle whenever getData() is null.
Do you want to provide a PR to fix the behaviour ?
getData()がnullのときは制御する必要があると思う。Pull Requestできる?(難しかったらこっちでやるよ)

くらいの意味でしょうか。せっかくなのでPull Requestしてみることにしました。

Pull Requestのガイドラインは、Submitting a Patchに説明があります。

ここでちょっと迷ったのが、Symfonyはバージョンごとにブランチが用意されているので、
どのブランチを修正すればよいか、ということです。

Symfonyのコミットログや他の人のPull Requestを確認してみると、該当機能が実装された一番下位のバージョンを修正しているようでした。下位バージョンにPull Requestしておけば、マージする際に、上位バージョンのブランチにも反映してくれるようです。

ということで、送ったPull Requestがこちらです。
https://github.com/symfony/symfony/pull/25102

Pull RequestもIssueと同様にテンプレートがあるので埋めていくだけです。

余談ですが、Pull Requestを投げるとほぼ同時にBugラベルやStatus:NeedReviewラベルが付けられます。Symfonyのレポジトリにはcarsonbotというbotが動いていて、レビューやマージがスムーズに進むよう自動化されているようです。

http://symfony.com/blog/calling-for-issue-triagers-a-new-workflow-and-the-carson-butler

Pull Requestもすぐに反応いただき、指摘事項を修正したらマージしてくれました。

マージされると、リリースノートにも名前が載ったりするようです。テンション上りますね!
https://github.com/symfony/symfony/releases/tag/v4.0.0-RC2

まとめ

SymfonyにPull Requestをするのは初めてだったのですが、全般的にとても反応が速く、コミュニティがうまく運営されているなあと感じました。

また、ドキュメントや自動化の仕組みなど、巨大なOSSだけあって、全体がスムーズにまわるようにとても工夫されています(とても勉強になりました)

もし不具合を見つけたりしたら、ぜひIssueの投稿やPull Requestにチャレンジしてみてください。一緒にSymfonyを良くしていければなあと思います!