phpcon沖縄2023_初めてのOSSコントリビュートに挑戦したら、既に該当機能が存在していた件について

by kero kero

Slide 1

Slide 1 text

初めての OSSコントリビュートに挑戦したら、  既に該当機能が存在していた件について    2023/09/16    (株)インフィニットループ　  サーバーサイドエンジニア　kerokero 

Slide 2

Slide 2 text

今回の発表の対象者  ・ OSSコントリビュートに興味がある初心者   

Slide 3

Slide 3 text

今回の発表の対象者  ・ OSSコントリビュートに興味がある初心者  ・ PHPやその周辺技術に貢献したい人   

Slide 4

Slide 4 text

今回の発表の対象者  ・ OSSコントリビュートに興味がある初心者  ・ PHPやその周辺技術に貢献したい人  ・ laravel/frameworkにPRを送ってみたい人 

Slide 5

Slide 5 text

今回の発表の対象者  ・ OSSコントリビュートに興味がある初心者  ・ PHPやその周辺技術に貢献したい人  ・ laravel/frameworkにPRを送ってみたい人  < 3つです 

Slide 6

Slide 6 text

Agenda  1.　軽い自己紹介    2.　laravel/framework にPRを出そうとした経緯    3.　実際にPRを出してみた    4.　それって〇〇と何が違うんだい？    5.　反省点・良かった点・感想 

Slide 7

Slide 7 text

Agenda  1.　軽い自己紹介　　　　　　　　　　    2.　laravel/framework にPRを出そうとした経緯    3.　実際にPRを出してみた    4.　それって〇〇と何が違うんだい？    5.　反省点・良かった点・感想 

Slide 8

Slide 8 text

どうも、kerokero です。 

Slide 9

Slide 9 text

22卒　自称青魔道士系エンジニア   （株）インフィニットループ   kerokero ・北海道の札幌生まれ、札幌育ち  ・水産→数学(非情報系)→2022年4月にIL入社　・最近の趣味は絵を描くこととストリートファイター6 です  ・担当領域はサーバーサイドのアプリ・インフラ実装(PHP, AWSなど) です  

Slide 10

Slide 10 text

Agenda  1.　軽い自己紹介　　　　　　　　　　    2.　laravel/framework にPRを出そうとした経緯    3.　実際にPRを出してみた    4.　それって〇〇と何が違うんだい？    5.　反省点・良かった点・感想 

Slide 11

Slide 11 text

というわけでいきなり本題ですが…… 

Slide 12

Slide 12 text

というわけでいきなり本題ですが……  Laravel のアップグレードをしようとしました   

Slide 13

Slide 13 text

Laravel のアップグレードをしようとしました    というわけでいきなり本題ですが……  ・チームで使っているLaravelのバージョンを9から10に移行することに    ・この作業は kerokero が担当することになった

Slide 14

Slide 14 text

すると…… 

Slide 15

Slide 15 text

psalm/plugin-laravel もアップデートされてた‼  すると……  ・Psalmプラグインの更新により、コードの静的解析の結果が変わった 

Slide 16

Slide 16 text

psalm/plugin-laravel もアップデートされてた‼  すると……  ・Psalmプラグインの更新により、コードの静的解析の結果が変わった  < なんとかして、大幅に実装を変えることなく静的解析が通るようにしたい......  

Slide 17

Slide 17 text

具体的には…… 

Slide 18

Slide 18 text

具体的には……  Illuminate/Support/Collection の get メソッドを使っている部分が大量に通らなくなってしまった ‼   

Slide 19

Slide 19 text

具体的には……  Illuminate/Support/Collection の get メソッドを使っている部分が大量に通らなくなってしまった ‼    ERROR: PossiblyNullPropertyFetch - app/Services/Queries/Weapon/WeaponQuery.php:87:19   - Cannot get property on possibly null variable $masterWeapon of type mixed|null (see https://psalm.dev/082)  ? $masterWeapon->id  $masterWeapon = $mstWeaponsByMaterial Count->get($masterByMaterial->materialCount);   psalm/plugin-laravel (v2.8.0)  

Slide 20

Slide 20 text

- Cannot get property on possibly null variable $masterWeapon of type mixed|null (see https://psalm.dev/082)  nullが入る可能性があるよって言われてる 

Slide 21

Slide 21 text

- Cannot get property on possibly null variable $masterWeapon of type mixed|null (see https://psalm.dev/082)  nullが入る可能性があるよって言われてる  null って mixed に含まれてるんじゃないの？　 >

Slide 22

Slide 22 text

そもそもアプリの仕様上 null が出ることはない...... 

Slide 23

Slide 23 text

そもそもアプリの仕様上 null が出ることはない......  null なら例外を吐けばよさそう！  < やったー！ 

Slide 24

Slide 24 text

first メソッドには、   firstOrFail メソッドがある…… 

Slide 25

Slide 25 text

first メソッドには、   firstOrFail メソッドがある……  一方、get メソッドには、   getOrFail メソッドがない……‼  まじかよ...... >

Slide 26

Slide 26 text

というわけで...... 

Slide 27

Slide 27 text

というわけで......  getOrFail メソッド追加すればいいんじゃない？

Slide 28

Slide 28 text

Agenda  1.　軽い自己紹介　　　　　　　　　　    2.　laravel/framework にPRを出そうとした経緯    3.　実際にPRを出してみた　　　　　　     4.　それって〇〇と何が違うんだい？    5.　反省点・良かった点・感想 

Slide 29

Slide 29 text

まずは laravel/framework への PR の出し方を調べる 

Slide 30

Slide 30 text

まずは laravel/framework への PR の出し方を調べる  ・OSS によって手順やルールが異なるので注意が必要    ・今回は Laravel が指定している方法を確認する必要がある 

Slide 31

Slide 31 text

まずは laravel/framework への PR の出し方を調べる  ・OSS によって手順やルールが異なるので注意が必要    ・今回は Laravel が指定している方法を確認する必要がある  AI いらすとや　すごい　 >

Slide 32

Slide 32 text

Laravel の場合は公式が Contribution Guide というものを出してる  ・https://laravel.com/docs/10.x/contributions   

Slide 33

Slide 33 text

Laravel の場合は公式が Contribution Guide というものを出してる  ・https://laravel.com/docs/10.x/contributions    ・ブランチの切り方や、コーディングスタイルの指定などがある 

Slide 34

Slide 34 text

Laravel の場合は公式が Contribution Guide というものを出してる  ・https://laravel.com/docs/10.x/contributions    ・ブランチの切り方や、コーディングスタイルの指定などがある    ・resources/css や resources/js など、他のコンパイル済みファイルに影響を与えるものに関しては、メンテナが別途生成しコミットする　　　　などなど

Slide 35

Slide 35 text

具体的な作業としては 

Slide 36

Slide 36 text

具体的な作業としては  ・laravel/frameworkをforkして、手元にcloneしてくるところから始める   

Slide 37

Slide 37 text

具体的な作業としては    ・laravel/frameworkをforkして、手元にcloneしてくるところから始める    ・手元で変更箇所を実装したら、Conventional Commitsというやり方に則ってコミットを積んであげる    fix: koko kesuto ugokanai >

Slide 38

Slide 38 text

Conventional Commits とは 

Slide 39

Slide 39 text

Conventional Commits とは  ・https://www.conventionalcommits.org/en/v1.0.0/    ・Conventional Commits というのは、コミットメッセージ上の規約のこと          

Slide 40

Slide 40 text

Conventional Commits とは  ・https://www.conventionalcommits.org/en/v1.0.0/    ・Conventional Commits というのは、コミットメッセージ上の規約のこと     ・feat(lang): add Polish language みたいなやつ       

Slide 41

Slide 41 text

Conventional Commits とは  ・https://www.conventionalcommits.org/en/v1.0.0/    ・Conventional Commits というのは、コミットメッセージ上の規約のこと     ・feat(lang): add Polish language みたいなやつ    ・セマンティックバージョニングに対応しており、自動化しやすい        

Slide 42

Slide 42 text

Slide 43

Slide 43 text

実際にコミットした内容について英語でメッセージを書いていきます   

Slide 44

Slide 44 text

実際にコミットした内容について英語でメッセージを書いていきます    ・日本語で書いて deepl を使い翻訳したものを確認したものを使いました     

Slide 45

Slide 45 text

実際にコミットした内容について英語でメッセージを書いていきます    ・日本語で書いて deepl を使い翻訳したものを確認したものを使いました     

Slide 46

Slide 46 text

実際にコミットした内容について英語でメッセージを書いていきます    ・日本語で書いて deepl を使い翻訳したものを確認したものを使いました      他の人のＰＲなども参考にしたよ >  

Slide 47

Slide 47 text

というわけでいよいよPR提出していくのですが…… 

Slide 48

Slide 48 text

というわけでいよいよPR提出していくのですが……  結論から行くと　うまくいきませんでした 

Slide 49

Slide 49 text

Agenda  1.　軽い自己紹介　　　　　　　　　　    2.　laravel/framework にPRを出そうとした経緯    3.　実際にPRを出してみた    4.　それって〇〇と何が違うんだい？　　　    5.　反省点・良かった点・感想 

Slide 50

Slide 50 text

実際にPRをだすと、GitHub上でCIが動き出します    ・各種指定したバージョンでのテストを通過することが必要だった

Slide 51

Slide 51 text

実際にPRをだすと、GitHub上でCIが動き出します    ・各種指定したバージョンでのテストを通過することが必要だった・なかなかテストが通らなかったので、手元で修正してコミットを積んでた

Slide 52

Slide 52 text

実際にPRをだすと、GitHub上でCIが動き出します    ・各種指定したバージョンでのテストを通過することが必要だった・なかなかテストが通らなかったので、手元で修正してコミットを積んでた・テストが通ったら squash してコミットをまとめようと思ってた

Slide 53

Slide 53 text

Slide 54

Slide 54 text

すると、まもなくしてレビューコメントをいただきました‼   

Slide 55

Slide 55 text

すると、まもなくしてレビューコメントをいただきました‼    それって sole メソッドと何が違うんだい？ 

Slide 56

Slide 56 text

すると、まもなくしてレビューコメントをいただきました‼    それって sole メソッドと何が違うんだい？  sole ...... ？ >

Slide 57

Slide 57 text

というわけで、sole メソッドについて調べてみた‼ 

Slide 58

Slide 58 text

というわけで、sole メソッドについて調べてみた‼ 

Slide 59

Slide 59 text

というわけで、sole メソッドについて調べてみた‼ 

Slide 60

Slide 60 text

というわけで、sole メソッドについて調べてみた‼  < たしかに、既にあったわ……  

Slide 61

Slide 61 text

Agenda  1.　軽い自己紹介　　　　　　　　　　    2.　laravel/framework にPRを出そうとした経緯    3.　実際にPRを出してみた    4.　それって〇〇と何が違うんだい？    5.　反省点・良かった点・感想　　　　 

Slide 62

Slide 62 text

今回の反省点・分かった点・感想

Slide 63

Slide 63 text

今回の反省点・分かった点・感想・事前に下調べを入念に重ねてから PR を出すべきだった       

Slide 64

Slide 64 text

今回の反省点・分かった点・感想・事前に下調べを入念に重ねてから PR を出すべきだった    ・内容はさておき、PRを出すこと自体はそれほど難しくないOSSも存在する       

Slide 65

Slide 65 text

今回の反省点・分かった点・感想・事前に下調べを入念に重ねてから PR を出すべきだった    ・内容はさておき、PRを出すこと自体はそれほど難しくないOSSも存在する    ・Laravelに限らずphp関連のOSSのコードを理解しようと思うと、どうしてもある程度php-srcの理解が必要になる   

Slide 66

Slide 66 text

今回の反省点・分かった点・感想・事前に下調べを入念に重ねてから PR を出すべきだった    ・内容はさておき、PRを出すこと自体はそれほど難しくないOSSも存在する    ・Laravelに限らずphp関連のOSSのコードを理解しようと思うと、どうしてもある程度php-srcの理解が必要になる    今回はissueや過去のPRも調べたつもりだったけど、そもそも既にある機能だというところが盲点だった

Slide 67

Slide 67 text

ご清聴ありがとうございました!! 