Upgrade to Pro — share decks privately, control downloads, hide ads and more …

DroidKaigi2023 突撃!隣のコードレビュー

sato
September 18, 2023

DroidKaigi2023 突撃!隣のコードレビュー

DroidKaigi2023で発表した「突撃!隣のコードレビュー 」のスライド資料

sato

September 18, 2023
Tweet

More Decks by sato

Other Decks in Research

Transcript

  1. 突撃!隣のコードレビュー
    created by Okumura / Sato Taichi

    View Slide

  2. スピーカー紹介
    合同会社DMM.com
    Androidエンジニア
    Okumura
    猫が好き
    Sato Taichi
     肉が好き

    View Slide

  3. コードレビューは好きですか?

    View Slide

  4. コードレビューのメリット
    不具合が混入するリスクの軽減
    可読性や保守性の向上
    メンバーのスキル向上
    など

    View Slide

  5. View Slide

  6. レビューに時間が

    取られすぎる…

    View Slide

  7. レビューに時間が

    取られすぎる…
    ギスギスする…

    View Slide

  8. レビューに時間が

    取られすぎる…
    ギスギスする…

    など、課題も発生

    View Slide

  9. ・弊社の開発体制について
    ・各チームの事例(3チーム)
     ・チーム概要
     ・レビューの流れ
     ・チームで工夫していること
     ・チームで改善したいこと
    ・個人の取り組み
     ・レビュアーとしての工夫
     ・レビューイとしての工夫
     ・Androidならではのレビュー観点
    話すこと(アジェンダ)

    View Slide

  10. ・弊社の開発体制について
    ・各チームの事例(3チーム)
     ・チーム概要
     ・レビューの流れ
     ・チームで工夫していること
     ・チームで改善したいこと
    ・個人の取り組み
     ・レビュアーとしての工夫
     ・レビューイとしての工夫
     ・Androidならではのレビュー観点
    話すこと(アジェンダ)

    View Slide

  11. ・弊社の開発体制について
    ・各チームの事例(3チーム)
     ・チーム概要
     ・レビューの流れ
     ・チームで工夫していること
     ・チームで改善したいこと
    ・個人の取り組み
     ・レビュアーとしての工夫
     ・レビューイとしての工夫
     ・Androidならではのレビュー観点
    話すこと(アジェンダ)

    View Slide

  12. セッションの目標

    View Slide

  13. セッションの目標
    コードレビューに対して
    ほんのちょっと
    前向きな気持ちに

    View Slide

  14. ・弊社の開発体制について
    ・各チームの事例(3チーム)
     ・チーム概要
     ・レビューの流れ
     ・チームで工夫していること
     ・チームで改善したいこと
    ・個人の取り組み
     ・レビュアーとしての工夫
     ・レビューイとしての工夫
     ・Androidならではのレビュー観点
    アジェンダ

    View Slide

  15. View Slide

  16. DMMポイント
    クラブ
    DMM TV DMMブックス
    開発チーム 開発チーム 開発チーム

    View Slide

  17. ・弊社の開発体制について
    ・各チームの事例(3チーム)
     ・チーム概要
     ・レビューの流れ
     ・チームで工夫していること
     ・チームで改善したいこと
    ・個人の取り組み
     ・レビュアーとしての工夫
     ・レビューイとしての工夫
     ・Androidならではのレビュー観点
    アジェンダ

    View Slide

  18. View Slide

  19. どんなアプリを作っているか?
    DMMポイントクラブ
    →お得にさまざまなDMMのサービスを

    利用できるようにする、

    ポイントプラットフォームアプリ

    View Slide

  20. チーム概要
    人数
     5名
    開発の特徴
    ・既存実装はAndroid Viewが多い
    ・新規実装や改修時にはJetpack Composeを採用

    View Slide

  21. レビューの流れ
    プルリク
    作成
    CI
    チェッ

    レビュー
    実施
    マージ
    チェックリストに沿って
    セルフチェック
    Approveがもらえるまで
    指摘→修正を繰り返す
    レビューしてくれた人全
    員からApproveをもらえ
    たらマージ
    問題が

    発生していたら対応

    View Slide

  22. チームとして工夫していること
    実装前に行う工夫
    ・実装前に設計のレビューで実装の流れを共有
    実装後に行う工夫
    ・プルリクエストのテンプレートを工夫
    ・チームとしてコードレビュータイムを設置

    View Slide

  23. DMMポイントクラブのプルリクテンプレート
    issueのリンク
    対応の概要
    変更前後のスクショ

    View Slide

  24. DMMポイントクラブのプルリクテンプレート
    設計レビューでも
    プルリクエストを作成
    セルフレビュー項目

    View Slide

  25. セルフレビュー項目
    テスト項目書も
    ここでセルフ
    チェック
    DMMポイントクラブのプルリクテンプレート

    View Slide

  26. トラッキングや
    パフォーマンス計測の
    実装についても
    セルフチェック項目を
    用意
    DMMポイントクラブのプルリクテンプレート

    View Slide

  27. コードレビュータイムについて
    元々は1回 30分のレビュータイムを1日に2回実施
    その時間で全員が集中的にレビューを行う
    ②15:00〜15:30
    ①11:00〜11:30
    勤務開始 勤務終了

    View Slide

  28. それでもレビュー工数がかさみ、
    各メンバーの週の目標タスクが達成できない…
    課題
    コードレビュータイムについて

    View Slide

  29. 午前のコードレビュータイムで
    口頭でのコミュニケーションを実施するように
    (例)・タスクの状況・プルリクエストの内容を説明
       ・タスクや実装についての困りごとを相談
    コードレビュータイムについて
    改善!

    View Slide

  30. 改善の結果…
    ・日が浅いメンバーの相談ハードルが下がった
    ・レビューを進めやすくなった
    ・コミュニケーション取りやすくなった
    コードレビュータイムについて

    View Slide

  31. これから改善したいこと
    模索
    スプリント終盤に溜まるレビューの消化

    View Slide

  32. スプリント終盤に溜まるレビューの消化
    偏りがちなレビュアーの分散
    チーム内におけるレビュー品質の均一化
    これから改善したいこと
    模索

    View Slide

  33. View Slide

  34. どんなアプリを作っているか?
    DMM TV
    →オリジナルの番組や

    漫画原作のドラマ、
    アニメが見放題のサービスを

    利用できるアプリ

    View Slide

  35. チーム概要
    人数
     8名
    コードの特徴
     KMP(Kotlin Multiplatform)で
     iOS/AndroidTVと一部を共有

    View Slide

  36. レビューの流れ
    プルリク
    作成
    CI
    チェッ

    レビュー
    実施
    マージ
    code ownerの承認が得
    られたらマージ
    特にみてほしいところ
    や対応issue、差分を
    確認する
    PFへの影響判断
    ラベルでauto
    assign
    CIで静的解析

    View Slide

  37. チームとして工夫していること
    1プルリクの修正量
     500行を目処
    ビルドチェック
     行いたい時にコマンドをプルリク上でコメントすることで
     各PFのビルドチェックを行う

    View Slide

  38. レビューのリマインド
     各PFのレビュー対象プルリクが多いため、

     open状態のプルリクがある場合は

     定期的にSlackでレビュアー向けにメンションを飛ばす
    チームとして工夫していること

    View Slide

  39. DMM TVのプルリクテンプレート
    レビュー観点
    issueチケット

    View Slide

  40. 変更デザイン
    プルリクで

    やったこと
    プルリクで

    やってないこと
    DMM TVのプルリクテンプレート

    View Slide

  41. 変更前後のスクショ
    影響範囲
    レビューイの

    最終確認項目
    DMM TVのプルリクテンプレート

    View Slide

  42. AndroidTV

    確認項目
    動作
    確認項目
    iOS
    確認項目
    DMM TVのプルリクテンプレート

    View Slide

  43. チームとして改善したいこと
    課題
    レビュアーの偏り
    特定分野の修正でレビュワーの偏りがある

    View Slide

  44. チームとして改善したいこと
    レビュアーの偏り
    特定分野の修正でレビュワーの偏りがある
    レビューされるプルリクの偏り
    修正内容がパッと解らないプルリクは放置され気味
    課題

    View Slide

  45. レビューされやすいプルリクを作るようにするを目標に

    「差分を500行以下にする」や

    「プルリクのスコープを狭める」などを行なっている。
    またプルリクに背景知識を書いておくなどを検討中。
    課題の解消
    改善中

    View Slide

  46. その他
    CI/CD環境
    iOSのビルド等を考慮に入れてBitriseを選定。
    今後はコスパを考えて

    定期的に調査することを視野に入れる。
    展望

    View Slide

  47. チームC(ブックス)

    View Slide

  48. DMMブックス
    電子書籍リーダー
    →DMMで購入した電子書籍を

    Android端末で閲覧するための

    公式ビューアアプリ
    どんなアプリを作っているか?

    View Slide

  49. チーム概要
    人数
     5名
    開発の特徴
     アジャイル開発
     ほぼKotlinでアーキテクチャはMVVM

    View Slide

  50. 開発の特徴
     現在はリファクタリングが比較的多いフェーズ
     JetpackComposeへの移行を視野に入れている
    チーム概要

    View Slide

  51. レビューの流れ
    プルリク
    作成
    CI
    チェッ

    レビュー
    実施
    マージ
    最低2人から
    Approveをもらえたら
    マージ
    Slackでレビュー依頼を
    投げる。
    レビュアーはチーム全員
    チェックリストに
    沿ってセルフ
    チェック。
    手元での動作確認
    中はDraftで作成
    問題が

    発生していたら対応

    View Slide

  52. チームとして工夫していること
    プルリクサイズの小型化
     平均変更行数200行前後。
     レビューにかかる時間が少なくて済む。
     観点が限定されて見逃しがしにくい。

    View Slide

  53. 設計を重めに取る
     設計段階で仮実装を利用して設計レビューを行い

     チーム内に相談して、レビュー観点や改修点を共有。
     設計では全体のプロセス説明やクラス図、

     変更前後のコードスニペットなどを記載する。
    チームとして工夫していること

    View Slide

  54. チームとして工夫していること
    レビューで起きる問題
    コードレビューで問題が起きる場合、

    実際には設計プロセスやコミュニケーションの不足などの

    レビューとは別の問題が、

    コードレビューで露呈して起きると考えています。

    View Slide

  55. レビュー工数の軽量化
     要検討項目は資料で合意済みの状態を作った上で、

     実装が資料通りの実装がなされているかを確認。
     更にレビューイがプルリクにコメント記載して

     意図を追記。
    チームとして工夫していること

    View Slide

  56. DMMブックスのプルリクテンプレート
    対応の概要
    プルリクの

    要点や観点

    View Slide

  57. 変更前後の
    スクショや動画
    対応の懸念点
    DMMブックスのプルリクテンプレート

    View Slide

  58. レビュアーが

    変更を確認するため
    の方法
    レビューイの

    最終確認項目
    DMMブックスのプルリクテンプレート

    View Slide

  59. チームとして改善したいこと
    1. 現在少人数で回しているので、

    人数が増えたらやり方が継続できるか不明瞭
    2. 動作確認の質を自動化などで均一化できておらず

    工数もそれなりにある
    課題

    View Slide

  60. QAチームと共同でシナリオテストを作成中。
    各画面の確認すべきことを文書化して、動作確認の際に品
    質を安定化させたい。
    レビュー品質の均一化
    改善中

    View Slide

  61. DMMポイントクラブ DMM TV DMMブックス
    取り組みについての小まとめ

    View Slide

  62. チームごとの取り組みについての小まとめ
    工夫
    プルリクは小さく作る
     プルリクの変更行数を出来るだけ少なくし
    て、 見やすいものにする
    CIをしっかり利用
     レビュー前にCIを利用して細かいミスは排除する

    View Slide

  63. チームごとの取り組みについての小まとめ
    課題
    レビュアーの偏り
     レビュー対応するレビュアーが偏ってしまう
    レビュー品質の偏り
     チーム内でのレビュー精度に偏りが出て

     品質が均一化出来ていない

    View Slide

  64. チームごとの取り組みについての小まとめ
    展望
    メンバーを教育
     実装やアーキテクチャの思想を

     設計やレビューを通して、

     メンバー全員が共通の認識を持てるようにしていく

    View Slide

  65. ・弊社の開発体制について
    ・各チームの事例(3チーム)
     ・チーム概要
     ・レビューの流れ
     ・チームで工夫していること
     ・チームで改善したいこと
    ・個人の取り組み
     ・レビュアーとしての工夫
     ・レビューイとしての工夫
     ・Androidならではのレビュー観点
    アジェンダ

    View Slide

  66. レビュアーとしての工夫

    View Slide

  67. よく挙がった観点
    ・認識をきちんと合わせる
    ・プルリクを記録やナレッジとして活用できるようにする
    ・スムーズなレビューを目指す
    ・心地よいコミュニケーションを心がける
    レビュアー(レビューする側)としての工夫

    View Slide

  68. レビュアー(レビューする側)としての工夫
    認識をきちんと合わせる

    View Slide

  69. レビュアー(レビューする側)としての工夫
    認識をきちんと合わせる
    ・ちょっとした疑問でも聞く

    View Slide

  70. レビュアー(レビューする側)としての工夫
    認識をきちんと合わせる
    ・ちょっとした疑問でも聞く
    ・プルリク上でのやりとりが3往復以上になりそうな場合は、

     対面レビューやモブレビューなどを実施し早期解決に努める

    View Slide

  71. レビュアー(レビューする側)としての工夫
    認識をきちんと合わせる
    ・ちょっとした疑問でも聞く
    ・プルリク上でのやりとりが3往復以上になりそうな場合は、

     対面レビューやモブレビューなどを実施し早期解決に努める
    ・コメントをした背景や理由をより具体的に明記しておく

    View Slide

  72. レビュアー(レビューする側)としての工夫
    記録やナレッジとしてプルリクを活用

    View Slide

  73. レビュアー(レビューする側)としての工夫
    記録やナレッジとしてプルリクを活用
    ・口頭で確認したこともプルリクのコメントで残す

    View Slide

  74. レビュアー(レビューする側)としての工夫
    記録やナレッジとしてプルリクを活用
    ・口頭で確認したこともプルリクのコメントで残す
    ・指摘に対して修正コミットを入れるだけではなく、

     返信・Resolveされているかまで確認する

    View Slide

  75. レビュアー(レビューする側)としての工夫
    記録やナレッジとしてプルリクを活用
    ・口頭で確認したこともプルリクのコメントで残す
    ・指摘に対して修正コミットを入れるだけではなく、

     返信・Resolveされているかまで確認する
    ・レビューコメントを読んだ人が今後のレビューをする・

     されるときに役立つように詳細に書く

    View Slide

  76. レビュアー(レビューする側)としての工夫
    スムーズなレビューを目指す

    View Slide

  77. レビュアー(レビューする側)としての工夫
    スムーズなレビューを目指す
    ・自分のタスク < コードレビュー

    View Slide

  78. レビュアー(レビューする側)としての工夫
    スムーズなレビューを目指す
    ・自分のタスク < コードレビュー
    ・なるべくすぐ見る

    View Slide

  79. (人やチームによって方針は要検討)

    レビュアー(レビューする側)としての工夫
    スムーズなレビューを目指す
    ・自分のタスク < コードレビュー
    ・なるべくすぐ見る
    ・指摘の優先度がわかりやすいように頭に分類をつけている

     →例:[MUST][IMO][nits]

    View Slide

  80. レビュアー(レビューする側)としての工夫
    心地よいコミュニケーションを心がける

    View Slide

  81. レビュアー(レビューする側)としての工夫
    心地よいコミュニケーションを心がける
    ・固すぎずゆるい感じの文体でコメントする

    View Slide

  82. レビュアー(レビューする側)としての工夫
    心地よいコミュニケーションを心がける
    ・固すぎずゆるい感じの文体でコメントする
    ・柔らかい口調にする

    View Slide

  83. レビュアー(レビューする側)としての工夫
    心地よいコミュニケーションを心がける
    ・固すぎずゆるい感じの文体でコメントする
    ・柔らかい口調にする
    ・自分の書き方の好みを押し付けない

    View Slide

  84. レビュアー(レビューする側)としての工夫
    心地よいコミュニケーションを心がける
    ・固すぎずゆるい感じの文体でコメントする
    ・柔らかい口調
    ・自分の書き方の好みを押し付けない
    ・指摘だけではなく、良いと思ったところもコメントする

    View Slide

  85. レビュアー(レビューする側)としての工夫
    その他

    View Slide

  86. レビュアー(レビューする側)としての工夫
    その他
    ・レビューしたコードを他の人にも説明できるくらいに責任を持つ

    View Slide

  87. レビュアー(レビューする側)としての工夫
    その他
    ・レビューしたコードを他の人にも説明できるくらいに責任を持つ
    ・自分が実装するならどうするかというのを意識して、

     違っている点を質問・コメントして自己学習やコード改善につなげている

    View Slide

  88. レビューイとしての工夫

    View Slide

  89. 差分のサイズを小さくする
    レビューイ(レビューされる側)としての工夫

    View Slide

  90. 差分のサイズを小さくする
    ・プルリクの粒度が大きくなりそうな場合、

     「テストは別プルリクで作成」など申し送り事項を設定しスコープを狭める
    レビューイ(レビューされる側)としての工夫

    View Slide

  91. 差分のサイズを小さくする
    ・プルリクの粒度が大きくなりそうな場合、

     「テストは別プルリクで作成」など申し送り事項を設定しスコープを狭める
    ・一瞬でわかるプルリクが理想なので、

     プルリクのスコープや影響範囲を狭くして、

     本筋以外の修正は含めない
    レビューイ(レビューされる側)としての工夫

    View Slide

  92. 差分のサイズを小さくできなくても・・・
    レビューイ(レビューされる側)としての工夫

    View Slide

  93. 差分のサイズを小さくできなくても・・・
    ・修正量が多い時はコミットを分けて、

     コミットを追うことで修正内容がわかりやすいようにする
    レビューイ(レビューされる側)としての工夫

    View Slide

  94. 差分のサイズを小さくできなくても・・・
    ・修正量が多い時はコミットを分けて、

     コミットを追うことで修正内容がわかりやすいようにする
    ・どうしても大きくならざるを得ないプルリクは、

     本当に見てほしいところをコメントでピックアップする
    レビューイ(レビューされる側)としての工夫

    View Slide

  95. 読みやすいプルリクにする
    レビューイ(レビューされる側)としての工夫

    View Slide

  96. 読みやすいプルリクにする
    ・意味のあるまとまりでプルリクを作成する

     (ドメイン層などのレイヤーごと・画面ごと・機能ごとなど)
    レビューイ(レビューされる側)としての工夫

    View Slide

  97. 読みやすいプルリクにする
    ・意味のあるまとまりでプルリクを作成する

     (ドメイン層などのレイヤーごと・画面ごと・機能ごとなど)
    ・処理の意図をコードコメントだけでなく、

     プルリクコメントでより具体的に補足する
    レビューイ(レビューされる側)としての工夫

    View Slide

  98. 必要な情報をきちんと揃える
    レビューイ(レビューされる側)としての工夫

    View Slide

  99. 必要な情報をきちんと揃える
    ・仕様や設計書のリンクなど、レビューに必要な材料を揃えてプルリクに書く
    レビューイ(レビューされる側)としての工夫

    View Slide

  100. 必要な情報をきちんと揃える
    ・仕様や設計書のリンクなど、レビューに必要な材料を揃えてプルリクに書く
    ・レビュアーが見たときに、

     合っている・合っていないの判断ができるように情報を整える
    レビューイ(レビューされる側)としての工夫

    View Slide

  101. 必要な情報をきちんと揃える
    ・仕様や設計書のリンクなど、レビューに必要な材料を揃えてプルリクに書く
    ・レビュアーが見たときに、

     合っている・合っていないの判断ができるように情報を整える
    ・プルリク上で動作確認のエビデンスとして動画を添付している
    レビューイ(レビューされる側)としての工夫

    View Slide

  102. その他
    ・指摘を修正後、コミットのリンクを付けて返信

     (指摘ごとにコミットを分ける)
    レビューイ(レビューされる側)としての工夫

    View Slide

  103. ・差分のサイズ
    ・コミットを分割
    ・意味のあるまとまり
    ・コメントで補足
    ・必要な情報を揃える
    etc…
    レビューイ(レビューされる側)としての工夫

    View Slide

  104. レビューイ(レビューされる側)としての工夫
    ・差分のサイズ
    ・コミットを分割
    ・意味のあるまとまり
    ・コメントで補足
    ・必要な情報を揃える
    etc…
    集まった意見はすべて
    「いかにレビューしやすくするか」
    にフォーカスしたもの

    View Slide

  105. レビューは
    コミュニケーションと
    思いやりめっちゃ大事

    View Slide

  106. Androidならではのレビュー観点

    View Slide

  107. iOSと比較して端末が多種多様(性能も画面サイズも)
    →パフォーマンス面、UI面の考慮が必要
    Androidならではのレビュー観点

    View Slide

  108. パフォーマンスについて
     ・メインセーフなつくりになっているか
     ・メモリリークを起こすところが無いか

      (LeakCanaryなどのツールを使う)
     ・レイアウトの入れ子構造が深くないか
    Androidならではのレビュー観点

    View Slide

  109. Androidならではのレビュー観点
    UIについて
     ・画面が小さい端末や、逆に大きい端末での表示が

      考慮できているか
     ・文字サイズや画面ズームの設定を変更しても崩れないか

    View Slide

  110. その他
     ・Activity破棄時・再生成時の挙動を考慮できているか
     ・OSバージョン間での挙動に問題がないか
    Androidならではのレビュー観点

    View Slide

  111. 〜アジェンダここまで〜

    View Slide

  112. セッションの目標の振り返り
    コードレビューに対して
    ほんのちょっと
    前向きな気持ちに

    View Slide

  113. Special Thanks
    ・インタビューに応じてくださった皆さん
    ・このスライドをレビューしてくださった皆さん
    ・セッションを聴いてくださった皆さん
    ありがとうございました!

    View Slide

  114. 最終ページ

    View Slide