Fusicきばんブログ

Fusic基盤ユニットの非公式技術ブログ

cakephp/cakephp にPR 出してmerge された話

(Fusic Advent Calendar 24日目)http://qiita.com/advent-calendar/2016/fusic & (CakePHP3 AdventCalendar 24日目)http://qiita.com/JoJeongMin/items/9fbc4cc1de9268b5de56http://qiita.com/advent-calendar/2016/cakephp3です。

Fusic 新卒の濱野です。
基盤ユニットに配属されてまもなく半年。
周りの席にはその道10年の職人さんが多く座っているので、
いつもつらい気持ちで席についています。

そんな肩身の狭い新卒ですが、
最近少しだけ胸をはれることがありました!

それは、、、

http://bakery.cakephp.org/2016/12/11/cakephp_3310_released.html
f:id:adiboy:20161223210812p:plain

そう。cakephp の core に出したPR が merge されたのです!!!
これはめでたい!!!

ということで、今回は、PR がマージされるまでの模様を時系列に沿ってお伝えしようと思います。
これを機に、PRの数が増えるとうれしいです!!!

何なおしたの?

pagination のバグを修正しました。

$paginate = [
    'limit' => 30,
    'maxLimit' => 20,
];

という設定をした時に、

maxLimit => 20

が無視され、

limit => 30

が採用されていました。
つまり、 maxLimit がちゃんと動いていませんでした。

どうやって見つけたか?

プラグインを作っていたら、挙動がおかしかったのでcore を読みました。
ちなみに、core は極力読むようにしています。
そして、時々ブログなどに結果報告を書いています。

例1
例2

その癖をつけていたおかげで、割とすんなり問題箇所の特定ができました。
そして、何が原因なのかがわかりました。

issue を登録する

こんなissue を登録しました。
https://github.com/cakephp/cakephp/issues/9848

全部英語で書くのが意外と大変でした。

PR を投げる

ついでに、issue に紐付ける形でPR も出しました。

I showed an example to fix this bug in #9849 ,
please review it, and I expect for some comments.
If my PullRequest is not worth to merge, don't hesitate to close it.

って書いて。
超意訳すると、

一応このissue を解決するPR 送りますー  

っていう感じです。

https://github.com/cakephp/cakephp/pull/9849

中の人とやり取りする

PR を出すとすぐ、chinpei215さんが調整してくださいました。
そして、マージされるまで面倒を見てくださいました。
この場を借りてもう一度お礼させて頂きます。
本当に有難うございました。

結局やり取りをしたのは2人でした。
一人は前述のchinpei215さん。
もう一人は、dereuromarkさん。

お二方とやり取りし、コードを修正しました。
そして、気づいたらマージされていました。

PR を出してから Merge されるまで1日もかかりませんでした。

感想

中の人は、優しかったです。(もちろんご指摘も頂きましたが)
そして、社内の人から声をかけられる機会が増えました。
これからも何かあったらどんどんプルリク出していこうと思います。

以上、未経験から入った新人が (cakephp/cakephp)https://github.com/cakephp/cakephp/ にPR を送ってマージされた話でした!