$30 off During Our Annual Pro Sale. View Details »

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

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

Tomoya Miwa

June 26, 2018
Tweet

More Decks by Tomoya Miwa

Other Decks in Programming

Transcript

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

    View Slide

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

    View Slide

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

    View Slide

  4. PRのきっかけ

    View Slide

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

    View Slide

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

    View Slide

  7. それがPermissionsDispatcher

    View Slide

  8. とりあえずissuesを⾒てみる

    View Slide

  9. View Slide

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

    View Slide

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

    View Slide

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

    View Slide

  13. あきらめることも⼤事

    View Slide

  14. 次いってみよう

    View Slide

  15. ⼆つめ
    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に対応していたのを初めて知った・・・

    View Slide

  16. 再現確認してみよう

    View Slide

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

    View Slide

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

    View Slide

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

    View Slide

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

    View Slide

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

    View Slide

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

    View Slide

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

    View Slide

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

    View Slide

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

    View Slide

  26. 祝︕マージ

    View Slide

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

    View Slide

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

    View Slide

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

    View Slide

  30. 今後

    View Slide

  31. 新たなるissue

    View Slide

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

    View Slide

  33. 楽勝じゃん︖と思ったら

    View Slide

  34. テストか必ず失敗する

    View Slide

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

    View Slide

  36. ようやくスタート地点にたてた︖

    View Slide

  37. ありがとうございました︕
    Let's send PR!

    View Slide