11
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 5 years have passed since last update.

SymfonyAdvent Calendar 2017

Day 11

SymfonyにPull Requestしてみました

Last updated at Posted at 2017-12-11

先日、たまたま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を参考に書いてみたのが以下です。

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

ちなみに英語は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が動いていて、レビューやマージがスムーズに進むよう自動化されているようです。

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

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

まとめ

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

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

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

11
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
11
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?