PermissionsDispatcherにPRをマージしてもらった話

by Tomoya Miwa

Slide 1

Slide 1 text

PermissionsDispatcherにPRをマージしてもらった話し tomoya0x00 shibuya.apk #26

Slide 2

Slide 2 text

簡単に⾃⼰紹介 tomoya0x00 Twitter, GitHub, Qiita Android, Embedded system, BLE/BT, iOS DeNA Co., Ltd. Automotive Business Unit.

Slide 3

Slide 3 text

初めてまともにOSSのPRを書いてマージしてもらったこれまでもtypo修正とかはあったんだけど… 皆様がPRを出すきっかけになれば技術的な話しはほとんどしません

Slide 4

Slide 4 text

PRのきっかけ

Slide 5

Slide 5 text

PRのきっかけ KotlinでStateMachineのライブラリを作ろうと思ったもしannotation processingでつくるならKotlinPoet触った⽅が良いかなーとつぶやいた

Slide 6

Slide 6 text

ある⼈から"KotlinPoetを使ってるプロジェクトが︕コントリどうです︖"と、replyが届いた

Slide 7

Slide 7 text

それがPermissionsDispatcher

Slide 8

Slide 8 text

とりあえずissuesを⾒てみる

Slide 9

Slide 9 text

No content

Slide 10

Slide 10 text

contributionwelcome タグがあるのでみてみた

Slide 11

Slide 11 text

⼀つめ Kotlin generation doesn't keep types of Array or ArrayList PermissionsDispatcherで⽣成したコードで引数の型が保持されないケースがある難しそう KotlinPoetに依存して起きているみたいな事がコメントに書かれている︖ PermissionsDispatcherの動きを知るためにも、がーっと⾒てみた

Slide 12

Slide 12 text

あきらめたどうやら現在はKotlinPoet的にも⼝が⽤意されてないのでできないっぽい調査する過程でPermissionsDispatcherの動きは何となくわかったのでOKとする

Slide 13

Slide 13 text

あきらめることも⼤事

Slide 14

Slide 14 text

次いってみよう

Slide 15

Slide 15 text

⼆つめ Fragment's onActivityResult() is not called when requesting SYSTEM_ALERT_WINDOW permission Fragmentで @NeedsPermission(Manifest.permission.SYSTEM_ALERT_WINDOW) の結果が受け取れない FragmentではなくActivityのonActivityResult()が呼ばれてしまう activity.startActivityForResult() -> fragment.startActivityForResult() とすれば良い SYSTEM_ALERT_WINDOWに対応していたのを初めて知った･･･

Slide 16

Slide 16 text

再現確認してみよう

Slide 17

Slide 17 text

そもそもビルドが通らない︕

Slide 18

Slide 18 text

そもそもビルドが通らない︕ ⽣成したコードで Fragment.getActivity().hoge みたいなコードがあるけど、 Fragment.getActivity() は nullable なので Support Library の v27 から︖ とりあえず !! で逃げるコード⽣成する側で nullable 判定して処理分岐はちょと⼤変そうだったので後回し

Slide 19

Slide 19 text

とりあえず、再現確認はできるようになった

Slide 20

Slide 20 text

ふと、コードを眺めていて思った

Slide 21

Slide 21 text

もしかしてこれ、Kotlin版だけの問題なんじゃね︖

Slide 22

Slide 22 text

もしかしてこれ、Kotlin版だけの問題なんじゃね︖ Java版とKotlin版は完全にソースが別どちらも基本的にはKotlinで書かれている Java版はJavaPoetでコード⽣成 Kotlin版はKotlinPoetでコード⽣成 issueで指摘されている問題、試して⾒るとJava版では発⽣しなかった

Slide 23

Slide 23 text

Java版をベースにKotlin版を修正

Slide 24

Slide 24 text

がっとPRつくってなげる WWDCのキーノート中にやってた･･･

Slide 25

Slide 25 text

さっそくレビューして貰えた !! の件は気になるけど、あとで対応しようねーという流れでマージして貰った

Slide 26

Slide 26 text

祝︕マージ

Slide 27

Slide 27 text

実際にやってみて - その１いつもとは違う脳みそを使った気がする初めて⾒るソースの理解には時間がかかるでも、進める内にある程度理解できるようになって楽しかった︕

Slide 28

Slide 28 text

実際にやってみて - その２英語ツラいがんばる Google翻訳も使うメインコントリビューターの⽅々は最悪⽇本語でやりとりできるのでハードル低いつぶやきに反応してくださったり

Slide 29

Slide 29 text

実際にやってみて - その３「オープンソースは別腹」という⾔葉が少し実感できた普段お世話になっているOSSに（少しでも）貢献できたのは嬉しい︕

Slide 30

Slide 30 text

今後

Slide 31

Slide 31 text

新たなるissue

Slide 32

Slide 32 text

新たなるissue Handling support fragment's getActivity() !! をどうにかしよう、というissue 要はnullチェックを追加すれば良い楽勝じゃん︖

Slide 33

Slide 33 text

楽勝じゃん︖と思ったら

Slide 34

Slide 34 text

テストか必ず失敗する

Slide 35

Slide 35 text

テストか必ず失敗する Java版はcompiling-testで実際にコード⽣成し、それのユニットテストを実施しているこのcompiling-testがWindowsだとうまく動かないことがわかった classpathの区切り⽂字がOSごとに異なるのが考慮されていないただし、headなら動く･･･が、リリースされていない