134
45

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 1 year has passed since last update.

お題は不問!Qiita Engineer Festa 2023で記事投稿!

僕が Laravel のコントリビューターになるまで

Last updated at Posted at 2023-07-05

この記事をぜひ読んでほしい人

  • OSS 活動をしてみたい人
  • 普段お世話になっているコミュニティに貢献したい人
  • 巨大 OSS に Pull Request を送る勇気が出ない人

OSS に Pull Request を送るのはつよつよエンジニアだけ

ずっとそう思っていました.
ましてや Laravel のような世界中で使われている巨大な OSS になんて,自分のようなジュニアエンジニアは Pull Request なんて送れない.送ってもすぐ Reject されて終わりだ,と考えていました.

一方で,普段から自分が仕事をできているのは OSS のおかげであるとも思っていたので,いつか自分もコミュニティに貢献できたらいいなぁと思っていました.

僕みたいに,OSS に Pull Request を送ってみたいけどなんとなく自信が持てない,と思っている方はたくさんいると感じるので,そういう方にもぜひ技術コミュニティへ参加に挑戦できるよう,自分の体験をシェアしようと思います.

Laravel にマージされた僕の Pull Request はたったの 2 件ですので,「Laravel のコントリビューター」と名乗るのはまだ早いかもしれませんが, Laravel 以外の海外製 OSS にもコントリビュートしているのでそこは大目に見ていただけると幸いです.

発端は僕の勘違いから

去年の話です.

僕は普段,本業・副業でコードレビューをすることもあるんですが,いつものようにコードレビューをしていて,このようなコードを発見しました.

とある FormRequest
public function rules(): array
{
    return [
        'foo' => ['required', 'regex:/^[a-zA-Z0-9]$/'],
    ];
}

ワイ「むむ,正規表現とな」
ワイ「半角アルファベットと数字なら,もともと Laravel にある alpha_num ルールで良さそうやな」
ワイ「コメントしておこう」
ワイ「"regex:/^[a-zA-Z0-9]$/ は alpha_num で良いと思いました" っと」

ーーー(次の日)ーーー

レビュイーはん「alpha_num だと全角文字を通してしまうので正規表現で絞ってます!」

ワイ「なんやて!?」
ワイ「alpha と num なんやからそんなわけないやろ!」
ワイ「実装みてみるか...」

当時の実際のコード
/**
 * Validate that an attribute contains only alpha-numeric characters.
 * 
 * @param  string  $attribute
 * @param  mixed  $value
 * @return bool
 */
public function validateAlphaNum($attribute, $value)
{
    if (! is_string($value) && ! is_numeric($value)) {
        return false;
    }

    return preg_match('/^[\pL\pM\pN]+$/u', $value) > 0;
} 

ワイ「むむ, \pL\pM\pNの部分,見慣れへんな...」
ワイ「調べてみるか」

image.png

ワイ「ふむ...」
ワイ「いやめっちゃ広いやんけ!!!!!!!」
ワイ「なにが an attribute contains only alpha-numeric characters やねん!」
ワイ「記号とか入っとるやんけ」

このバリデーションルールを僕自身が使ったことなく,単なる知識として知っていただけだったため,実際の挙動を確認して驚きました.
たしかに,このような実装であるなら,正規表現や,カスタムバリデーションを作るのは妥当だと感じ,その時は納得しました.

でもこのとき,僕はこう思いました.

ワイ「これ,命名が悪いなぁ」
ワイ「日本人絶対勘違いするやんけ」
ワイ「もしかして,言語圏によって ”アルファベット” の認識が違うんだろうか」
ワイ「マルチバイトな言語圏だと,アルファベットは [a-zA-Z] やけどなぁ」
ワイ「これ,もしかしてアルファベットの定義をしっかり示せば Pull Request 通るかも...?」

これが,僕が Laravel に初めて Pull Request を出すことになったきっかけです.

とりあえず勇気を出して PR を投げてみた

いろいろ思考錯誤した結果,今あるメソッドを変えてしまうのは破壊的変更であると判断し,シングルバイト文字だけを通す新しいメソッドを作ることにしました.

この PR の趣旨を簡単にまとめるとこうです.

  • alpha_num というルールは,値が英数字かどうかをチェックするものである
  • しかし,日本のようなマルチバイト圏では全角英数字とかもある
  • Validation で半角英数字だけを通すルールがあるとより良いと思う
  • つまり ASCII 文字のうち,英数字のみを通すルールを作りたい
  • 正規表現でも実現できるが,ルールとして存在していたほうが使いやすい

ドキドキしながらこの PR を Open しましたが,このような返信と共に Close されてしまいました.

image.png

Taylorはん「自分でカスタムルール作ってや!」

ワイ「何も分かっとらんな!」

なんとしてもこの変更を受け入れてもらいたい

あまり,マルチバイト圏に暮らすエンジニアのツラミが伝わらなかったようです.
でも,僕は引き下がれなかったため,一度コミュニティに問いかけてみることにしました.
そのときに使った手段が, GitHub の Discussions です.
Discussions は,オープンソースプロジェクト内のコミュニティで,ユーザー同士がコミュニケーションできる場です.
ここでは,質問をしたり情報共有をしたり,コミュニティのトピックの範囲内で自由に議論を交わすことができます.

僕はここで,他のマルチバイト圏のエンジニア,シングルバイト圏のエンジニアが「アルファベット」についてどう考えているのか聞いてみることにしました.
ここで賛同する意見をもらったり, Upvote してもらうことで,問題の深刻さを Taylor にアピールしようと考えたのです.

この Discussion を作るに当たって,工夫した点がありました.それは,できるだけ多くの”広く知られた”資料を引用することです.
たとえば,今回は 「ASCII 文字」や「Alphabet」に関するトピックだったので,

このように,自分の意見だけではなく,一般的な認識や定義,同様の意見などを提示しながら話を展開することで,より説得力を得ることができると思います.
結果として,返信こそアジア圏のエンジニアからしかもらえなかったものの,賛同意見をいただくことができました.
そして, Upvote 数は Laravel Discussions 全体で 2 番目に多い 35 票をもらえました.

満を持して 2 回目の PR

正直,このときは調子に乗っていました.

ワイ「Discussion でめっちゃ賛同を得られたわ」
ワイ「これはワイの勝ちですわ」
ワイ「破壊的変更やけど,元からあるメソッドを修正したるわ!」

結果👇

image.png

ごめんね Taylor.ワイが悪かった.

Taylor 氏は,前回の PR よりはこの変更の趣旨を受け入れてくれたようですが,やはり長くから使われているこのルールをガラッと変えてしまうのは,あまりに破壊的だそうです.
僕もそう思います.
かわりに Taylor 氏は,こんな助言をしてくれました

I would actually leave the current rules as they are and instead introduce ascii_alpha variants

Taylorはん「ワイならこのルールはそのままにして, ascii_alpha を別に実装するかな」

ごもっとも.同意見です.
そして優しいですね,代案を Laravel の作者が提示してくれました.
流れは来ています.後少し.次の PR できっと受け入れてもらえる...!

次こそ満を持しました. 3 回目の PR

前回の Taylor 氏の助言通り,もとある alpha_* ルールはそのままにした上で, 新たに

  • ascii_alpha
  • ascii_alpha_dash
  • ascii_alpha_num

ルールを追加しました.

すると,別の方からこんなレビューをいただきました.

Instead of creating new validation rules, it would be better to add an optional ascii option to the existing ones: alpha:ascii, alpha_dash:ascii, alpha_num:ascii

tpetry はん「新しいルールを作るかわりに,既存ルールに :ascii オプションを作ったら?」

はい天才です.
顧客が本当に求めていたものです.

無事マージ 🎉

tpetry 氏の提案には, Taylor 氏も賛同していて,めでたく :ascii オプション追加という形で僕の PR がマージされました.
この変更は Laravel 9.49 から取り込まれ,現在最新版の Laravel でもお使いいただけます.

Laravel に初コントリビュートしてみて

今回の一連の出来事は,自分の中で大きな学びになりました.
最初は,ただ「自分が正しいと思う定義に実装を寄せたい」の一心で PR を作りましたが,その主張がなかなか理解してもらえませんでした.
しかしながら,伝え方を工夫したり,アプローチを変えてみたり,コミュニティ全体に一度とかけてみたりすることで,自分がどのような意図でその変更をフレームワークに施したいのかを伝えることができたと思います.

また,世界で使われるフレームワークをメンテしていくにあたって,なんでも正しいと思った方向に修正していくことが正解とは限らないということも学びました.
「Backward Compatibility」 という言葉がありますが,既にその機能を”正しいものとして”利用している人が多くいる中では,結果が大きく変わるような破壊的変更をすることは多大な影響を及ぼすので,慎重にならなければなりません.
もちろん,我々が普段仕事でおこなている一般的なアプリケーション開発においても,破壊的な変更に対しては慎重でなければなりませんが,Laravel 規模の OSS だとなおさら,一つの変更で大きな影響を与えてしまうことを意識しなければいけません.

今回,

  1. 新たにメソッドを実装する(新規追加)
  2. 既にあるメソッドを修正する(破壊的変更)
  3. 既にあるメソッドに Optional な選択肢を追加する(非破壊的変更)

という風に問題解決のアプローチを変えて,何度かリトライしたわけですが,最終的には Taylor 氏や tpetry 氏のアドバイスを受け入れる形に落ち着きました.
最初の段階では自分の考慮や実装力が足りていなかったのかもしれませんが,メンテナのディレクションによって最終的には修正を完了できたという点で,あまり億劫になりすぎずとりあえず挑戦してみて良いんだな,という考えに至りました.
もちろん,雑な PR を投げて良いというわけでは決してありませんが,自分の技術力が足りていないことを理由に挑戦を諦めるべきではないと思っています.

最後に

自分が提案した修正が,世界規模の巨大なオープンソースプロジェクトの一部となる瞬間は,純粋に嬉しかったです.そして,日々自分が利用しているソフトウェアへ自分自身が貢献できたという事実は,とても誇らしい経験でした.

OSSはその本質的にコミュニティによる支援が必要不可欠であり,その一部として活動できることは,非常に魅力的な経験となります.もしも自分が使っている技術が心から好きで,自分のスキルを使って何か貢献したいと思っているなら,OSSへの参加はきっと有益な経験となると思います.そして,そんな経験をぜひとも他の技術者たちにもオススメしたいと思います.

134
45
3

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
134
45

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?