ossにPR送ったら添削してもらってマージされた話
こんにちは。calloc134です。
つい先日、ossに送ったPRがマージされました。
ossに対するPRのマージは初めてだったので、とても嬉しかったです。
今回は、その話をしていこうと思います。
はじめに
今回PRを送ったossは、ladleというツールです。
このツールはStorybookの軽量版のようなもので、Storybookのようにコンポーネントを管理することができます。
Storybookは非常に便利なコンポーネント管理ツールですが、多機能すぎるため、使いこなすのが難しいという問題があります。また、多機能である分、重いという問題もあります。
ladleは、Storybookのようなコンポーネント管理ツールを、より軽量にしたものです。Vite環境で動作し、Storybookのようにコンポーネントを管理することができます。Storybookと同様に、コンポーネントのドキュメントを書くことができます。
今回のPR
今回のPRは、このladleに対して送りました。
このPRでは、ladleの型定義が間違ったフィールドに存在したため、その修正を行ったという、内容としては簡単なものです。
利用している際に発見し、公式のドキュメント通りに修正したら動作したのを見て、型定義の誤りだと気づきました。
最初のPRとその失敗
このPR以前にも同じ内容でPRを送っていました。
しかし、PRを見てもらえばわかる通り、このPRは自分からクローズしています。
これを作成したとき、手元の環境でのテストをおこなわずにPRを送ってしまいました。そのため、CIでテストが失敗した状態となってしまいました。
今考えると、しっかり以下のコントリビュートガイドを読んでいれば、このような失敗はしなかったと思います。
PRに対する会話
このPRに対して、
- 型をもっと厳密にすべきか
- storybookのAPIとの互換性を持たせるようにするべきか
- e2eテストの内容を書き換えるのは避けるべきか
のような会話を行いました。
また、これに伴って// @ts-expect-errorを削除できるため、削除するかどうかの会話も行いました。
結果、型自体は厳格化せず、K[] | unknownのような合併型を利用することにしました。また、// @ts-expect-errorは削除することにしました。
これであっても、誤ったフィールド設定の時に比べると、型が確認できるため、開発者はより早く問題を発見できるようになります。
素人のPRでもしっかり分析をし、添削をしていただいた開発者の方には感謝しかありません。
(changesetの使い方がいまいちわからず、majorで送ってしまっているのをpatchにするように指摘していただいてます)
最終的な変更点
最終的に、以下のような変更点となりました。
型を厳格化することはせず、K[] | unknownのような合併型を利用することにし、// @ts-expect-errorは削除することにしてあります。
このPRを送信して三か月後、マージされました。
忘れていたころにふとgmailで通知がきて、マージされていることを知りました。驚いた・・・
リリースノートにも名前が載っています。なんかすげ～～
まとめ
今回は、ossにPRを送った話をしました。
内容は型定義の修正でしたが、それでもossに対してPRを送るのは初めてだったので、マージされてとても嬉しかったです。
自分はあまり知られていないライブラリを使うのが好きで、今回のPR箇所も自分が使っているライブラリの一つでした。
皆さんも、あまり知られていないライブラリで心が躍るものがあれば積極的に使ってみて、そして是非PRを送ってみてください。
PS. 今度からコントリビュートガイドはちゃんと読みます・・・
Discussion