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

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

Transcript

1. 突撃！隣のコードレビュー created by Okumura / Sato Taichi
2. スピーカー紹介 合同会社DMM.com Androidエンジニア Okumura 猫が好き Sato Taichi 　肉が好き
3. コードレビューは好きですか？
4. コードレビューのメリット
   不具合が混入するリスクの軽減
   可読性や保守性の向上
   メンバーのスキル向上 など
6. レビューに時間が
 取られすぎる…
7. レビューに時間が
 取られすぎる… ギスギスする…
8. レビューに時間が
 取られすぎる… ギスギスする… … など、課題も発生
9. ・弊社の開発体制について ・各チームの事例（3チーム） 　・チーム概要 　・レビューの流れ 　・チームで工夫していること 　・チームで改善したいこと ・個人の取り組み 　・レビュアーとしての工夫 　・レビューイとしての工夫 　・Androidならではのレビュー観点 話すこと（アジェンダ）
10. ・弊社の開発体制について ・各チームの事例（3チーム） 　・チーム概要 　・レビューの流れ 　・チームで工夫していること 　・チームで改善したいこと ・個人の取り組み 　・レビュアーとしての工夫 　・レビューイとしての工夫 　・Androidならではのレビュー観点 話すこと（アジェンダ）
11. ・弊社の開発体制について ・各チームの事例（3チーム） 　・チーム概要 　・レビューの流れ 　・チームで工夫していること 　・チームで改善したいこと ・個人の取り組み 　・レビュアーとしての工夫 　・レビューイとしての工夫 　・Androidならではのレビュー観点 話すこと（アジェンダ）
12. セッションの目標
13. セッションの目標 コードレビューに対して ほんのちょっと 前向きな気持ちに
14. ・弊社の開発体制について ・各チームの事例（3チーム） 　・チーム概要 　・レビューの流れ 　・チームで工夫していること 　・チームで改善したいこと ・個人の取り組み 　・レビュアーとしての工夫 　・レビューイとしての工夫 　・Androidならではのレビュー観点 アジェンダ
16. DMMポイント クラブ DMM TV DMMブックス 開発チーム 開発チーム 開発チーム
17. ・弊社の開発体制について ・各チームの事例（3チーム） 　・チーム概要 　・レビューの流れ 　・チームで工夫していること 　・チームで改善したいこと ・個人の取り組み 　・レビュアーとしての工夫 　・レビューイとしての工夫 　・Androidならではのレビュー観点 アジェンダ
19. どんなアプリを作っているか？ DMMポイントクラブ →お得にさまざまなDMMのサービスを 利用できるようにする、 ポイントプラットフォームアプリ
20. チーム概要 人数 　5名 開発の特徴 ・既存実装はAndroid Viewが多い ・新規実装や改修時にはJetpack Composeを採用
21. レビューの流れ プルリク 作成 CI チェッ ク レビュー 実施 マージ チェックリストに沿って セルフチェック Approveがもらえるまで 指摘→修正を繰り返す レビューしてくれた人全 員からApproveをもらえ たらマージ 問題が
 発生していたら対応
22. チームとして工夫していること 実装前に行う工夫 ・実装前に設計のレビューで実装の流れを共有 実装後に行う工夫 ・プルリクエストのテンプレートを工夫 ・チームとしてコードレビュータイムを設置
23. DMMポイントクラブのプルリクテンプレート issueのリンク 対応の概要 変更前後のスクショ
24. DMMポイントクラブのプルリクテンプレート 設計レビューでも プルリクエストを作成 セルフレビュー項目
25. セルフレビュー項目 テスト項目書も ここでセルフ チェック DMMポイントクラブのプルリクテンプレート
26. トラッキングや パフォーマンス計測の 実装についても セルフチェック項目を 用意 DMMポイントクラブのプルリクテンプレート
27. コードレビュータイムについて 元々は1回 30分のレビュータイムを1日に2回実施 その時間で全員が集中的にレビューを行う ②15:00〜15:30 ①11:00〜11:30 勤務開始 勤務終了
28. それでもレビュー工数がかさみ、 各メンバーの週の目標タスクが達成できない… 課題 コードレビュータイムについて
29. 午前のコードレビュータイムで 口頭でのコミュニケーションを実施するように （例）・タスクの状況・プルリクエストの内容を説明 　　　・タスクや実装についての困りごとを相談 コードレビュータイムについて 改善！
30. 改善の結果… ・日が浅いメンバーの相談ハードルが下がった ・レビューを進めやすくなった ・コミュニケーション取りやすくなった コードレビュータイムについて
31. これから改善したいこと 模索
    スプリント終盤に溜まるレビューの消化
32. 
    スプリント終盤に溜まるレビューの消化
    偏りがちなレビュアーの分散
    チーム内におけるレビュー品質の均一化 これから改善したいこと 模索
34. どんなアプリを作っているか？ DMM TV →オリジナルの番組や
 漫画原作のドラマ、 アニメが視聴可能な動画配信サービスを
 利用できるアプリ
35. チーム概要 人数 　8名 コードの特徴 　KMP（Kotlin Multiplatform）で 　iOS/AndroidTVと一部を共有
36. レビューの流れ プルリク 作成 CI チェッ ク レビュー 実施 マージ code ownerの承認が得 られたらマージ 特にみてほしいところ や対応issue、差分を 確認する PFへの影響判断 ラベルでauto assign CIで静的解析
37. チームとして工夫していること 1プルリクの修正量 　500行を目処 ビルドチェック 　行いたい時にコマンドをプルリク上でコメントすることで 　各PFのビルドチェックを行う
38. レビューのリマインド 　各PFのレビュー対象プルリクが多いため、
 　open状態のプルリクがある場合は
 　定期的にSlackでレビュアー向けにメンションを飛ばす チームとして工夫していること
39. DMM TVのプルリクテンプレート レビュー観点 issueチケット
40. 変更デザイン プルリクで
 やったこと プルリクで
 やってないこと DMM TVのプルリクテンプレート
41. 変更前後のスクショ 影響範囲 レビューイの
 最終確認項目 DMM TVのプルリクテンプレート
42. AndroidTV
 確認項目 動作 確認項目 iOS 確認項目 DMM TVのプルリクテンプレート
43. チームとして改善したいこと 課題 レビュアーの偏り 特定分野の修正でレビュワーの偏りがある
44. チームとして改善したいこと レビュアーの偏り 特定分野の修正でレビュワーの偏りがある レビューされるプルリクの偏り 修正内容がパッと解らないプルリクは放置され気味 課題
45. レビューされやすいプルリクを作るようにするを目標に
 「差分を500行以下にする」や
 「プルリクのスコープを狭める」などを行なっている。 またプルリクに背景知識を書いておくなどを検討中。 課題の解消 改善中
46. その他 CI/CD環境 iOSのビルド等を考慮に入れてBitriseを選定。 今後はコスパを考えて
 定期的に調査することを視野に入れる。 展望
47. チームC（ブックス）
48. DMMブックス 電子書籍リーダー →DMMブックスで購入した電子書籍を
 Android端末で閲覧するための
 公式ビューアアプリ どんなアプリを作っているか？
49. チーム概要 人数 　5名 開発の特徴 　アジャイル開発 　ほぼKotlinでアーキテクチャはMVVM
50. 開発の特徴 　現在はリファクタリングが比較的多いフェーズ 　JetpackComposeへの移行を視野に入れている チーム概要
51. レビューの流れ プルリク 作成 CI チェッ ク レビュー 実施 マージ 最低2人から Approveをもらえたら マージ Slackでレビュー依頼を 投げる。 レビュアーはチーム全員 チェックリストに 沿ってセルフ チェック。 手元での動作確認 中はDraftで作成 問題が
 発生していたら対応
52. チームとして工夫していること プルリクサイズの小型化 　平均変更行数200行前後。 　レビューにかかる時間が少なくて済む。 　観点が限定されて見逃しがしにくい。
53. 設計を重めに取る 　設計段階で仮実装を利用して設計レビューを行い
 　チーム内に相談して、レビュー観点や改修点を共有。 　設計では全体のプロセス説明やクラス図、
 　変更前後のコードスニペットなどを記載する。 チームとして工夫していること
54. チームとして工夫していること レビューで起きる問題 コードレビューで問題が起きる場合、
 実際には設計プロセスやコミュニケーションの不足などの
 レビューとは別の問題が、
 コードレビューで露呈して起きると考えています。
55. レビュー工数の軽量化 　要検討項目は資料で合意済みの状態を作った上で、
 　実装が資料通りの実装がなされているかを確認。 　更にレビューイがプルリクにコメント記載して
 　意図を追記。 チームとして工夫していること
56. DMMブックスのプルリクテンプレート 対応の概要 プルリクの
 要点や観点
57. 変更前後の スクショや動画 対応の懸念点 DMMブックスのプルリクテンプレート
58. レビュアーが
 変更を確認するため の方法 レビューイの
 最終確認項目 DMMブックスのプルリクテンプレート
59. チームとして改善したいこと 1. 現在少人数で回しているので、
 人数が増えたらやり方が継続できるか不明瞭 2. 動作確認の質を自動化などで均一化できておらず
 工数もそれなりにある 課題
60. QAチームと共同でシナリオテストを作成中。 各画面の確認すべきことを文書化して、動作確認の際に品 質を安定化させたい。 レビュー品質の均一化 改善中
61. DMMポイントクラブ DMM TV DMMブックス 取り組みについての小まとめ
62. チームごとの取り組みについての小まとめ 工夫 プルリクは小さく作る 　プルリクの変更行数を出来るだけ少なくし て、　見やすいものにする CIをしっかり利用 　レビュー前にCIを利用して細かいミスは排除する
63. チームごとの取り組みについての小まとめ 課題 レビュアーの偏り 　レビュー対応するレビュアーが偏ってしまう レビュー品質の偏り 　チーム内でのレビュー精度に偏りが出て
 　品質が均一化出来ていない
64. チームごとの取り組みについての小まとめ 展望 メンバーを教育 　実装やアーキテクチャの思想を
 　設計やレビューを通して、
 　メンバー全員が共通の認識を持てるようにしていく
65. ・弊社の開発体制について ・各チームの事例（3チーム） 　・チーム概要 　・レビューの流れ 　・チームで工夫していること 　・チームで改善したいこと ・個人の取り組み 　・レビュアーとしての工夫 　・レビューイとしての工夫 　・Androidならではのレビュー観点 アジェンダ
66. レビュアーとしての工夫
67. よく挙がった観点 ・認識をきちんと合わせる ・プルリクを記録やナレッジとして活用できるようにする ・スムーズなレビューを目指す ・心地よいコミュニケーションを心がける レビュアー（レビューする側）としての工夫
68. レビュアー（レビューする側）としての工夫 認識をきちんと合わせる
69. レビュアー（レビューする側）としての工夫 認識をきちんと合わせる ・ちょっとした疑問でも聞く
70. レビュアー（レビューする側）としての工夫 認識をきちんと合わせる ・ちょっとした疑問でも聞く ・プルリク上でのやりとりが3往復以上になりそうな場合は、
 　対面レビューやモブレビューなどを実施し早期解決に努める
71. レビュアー（レビューする側）としての工夫 認識をきちんと合わせる ・ちょっとした疑問でも聞く ・プルリク上でのやりとりが3往復以上になりそうな場合は、
 　対面レビューやモブレビューなどを実施し早期解決に努める ・コメントをした背景や理由をより具体的に明記しておく
72. レビュアー（レビューする側）としての工夫 記録やナレッジとしてプルリクを活用
73. レビュアー（レビューする側）としての工夫 記録やナレッジとしてプルリクを活用 ・口頭で確認したこともプルリクのコメントで残す
74. レビュアー（レビューする側）としての工夫 記録やナレッジとしてプルリクを活用 ・口頭で確認したこともプルリクのコメントで残す ・指摘に対して修正コミットを入れるだけではなく、
 　返信・Resolveされているかまで確認する
75. レビュアー（レビューする側）としての工夫 記録やナレッジとしてプルリクを活用 ・口頭で確認したこともプルリクのコメントで残す ・指摘に対して修正コミットを入れるだけではなく、
 　返信・Resolveされているかまで確認する ・レビューコメントを読んだ人が今後のレビューをする・
 　されるときに役立つように詳細に書く
76. レビュアー（レビューする側）としての工夫 スムーズなレビューを目指す
77. レビュアー（レビューする側）としての工夫 スムーズなレビューを目指す ・自分のタスク ＜ コードレビュー
78. レビュアー（レビューする側）としての工夫 スムーズなレビューを目指す ・自分のタスク ＜ コードレビュー ・なるべくすぐ見る
79. （人やチームによって方針は要検討） ｝ レビュアー（レビューする側）としての工夫 スムーズなレビューを目指す ・自分のタスク ＜ コードレビュー ・なるべくすぐ見る ・指摘の優先度がわかりやすいように頭に分類をつけている
 　→例：[MUST][IMO][nits]
80. レビュアー（レビューする側）としての工夫 心地よいコミュニケーションを心がける
81. レビュアー（レビューする側）としての工夫 心地よいコミュニケーションを心がける ・固すぎずゆるい感じの文体でコメントする
82. レビュアー（レビューする側）としての工夫 心地よいコミュニケーションを心がける ・固すぎずゆるい感じの文体でコメントする ・柔らかい口調にする
83. レビュアー（レビューする側）としての工夫 心地よいコミュニケーションを心がける ・固すぎずゆるい感じの文体でコメントする ・柔らかい口調にする ・自分の書き方の好みを押し付けない
84. レビュアー（レビューする側）としての工夫 心地よいコミュニケーションを心がける ・固すぎずゆるい感じの文体でコメントする ・柔らかい口調 ・自分の書き方の好みを押し付けない ・指摘だけではなく、良いと思ったところもコメントする
85. レビュアー（レビューする側）としての工夫 その他
86. レビュアー（レビューする側）としての工夫 その他 ・レビューしたコードを他の人にも説明できるくらいに責任を持つ
87. レビュアー（レビューする側）としての工夫 その他 ・レビューしたコードを他の人にも説明できるくらいに責任を持つ ・自分が実装するならどうするかというのを意識して、
 　違っている点を質問・コメントして自己学習やコード改善につなげている
88. レビューイとしての工夫
89. 差分のサイズを小さくする レビューイ（レビューされる側）としての工夫
90. 差分のサイズを小さくする ・プルリクの粒度が大きくなりそうな場合、
 　「テストは別プルリクで作成」など申し送り事項を設定しスコープを狭める レビューイ（レビューされる側）としての工夫
91. 差分のサイズを小さくする ・プルリクの粒度が大きくなりそうな場合、
 　「テストは別プルリクで作成」など申し送り事項を設定しスコープを狭める ・一瞬でわかるプルリクが理想なので、
 　プルリクのスコープや影響範囲を狭くして、
 　本筋以外の修正は含めない レビューイ（レビューされる側）としての工夫
92. 差分のサイズを小さくできなくても・・・ レビューイ（レビューされる側）としての工夫
93. 差分のサイズを小さくできなくても・・・ ・修正量が多い時はコミットを分けて、
 　コミットを追うことで修正内容がわかりやすいようにする レビューイ（レビューされる側）としての工夫
94. 差分のサイズを小さくできなくても・・・ ・修正量が多い時はコミットを分けて、
 　コミットを追うことで修正内容がわかりやすいようにする ・どうしても大きくならざるを得ないプルリクは、
 　本当に見てほしいところをコメントでピックアップする レビューイ（レビューされる側）としての工夫
95. 読みやすいプルリクにする レビューイ（レビューされる側）としての工夫
96. 読みやすいプルリクにする ・意味のあるまとまりでプルリクを作成する
 　(ドメイン層などのレイヤーごと・画面ごと・機能ごとなど) レビューイ（レビューされる側）としての工夫
97. 読みやすいプルリクにする ・意味のあるまとまりでプルリクを作成する
 　(ドメイン層などのレイヤーごと・画面ごと・機能ごとなど) ・処理の意図をコードコメントだけでなく、
 　プルリクコメントでより具体的に補足する レビューイ（レビューされる側）としての工夫
98. 必要な情報をきちんと揃える レビューイ（レビューされる側）としての工夫
99. 必要な情報をきちんと揃える ・仕様や設計書のリンクなど、レビューに必要な材料を揃えてプルリクに書く レビューイ（レビューされる側）としての工夫
100. 必要な情報をきちんと揃える ・仕様や設計書のリンクなど、レビューに必要な材料を揃えてプルリクに書く ・レビュアーが見たときに、
 　合っている・合っていないの判断ができるように情報を整える レビューイ（レビューされる側）としての工夫
101. 必要な情報をきちんと揃える ・仕様や設計書のリンクなど、レビューに必要な材料を揃えてプルリクに書く ・レビュアーが見たときに、
 　合っている・合っていないの判断ができるように情報を整える ・プルリク上で動作確認のエビデンスとして動画を添付している レビューイ（レビューされる側）としての工夫
102. その他 ・指摘を修正後、コミットのリンクを付けて返信
 　（指摘ごとにコミットを分ける） レビューイ（レビューされる側）としての工夫
103. ・差分のサイズ ・コミットを分割 ・意味のあるまとまり ・コメントで補足 ・必要な情報を揃える etc… レビューイ（レビューされる側）としての工夫
104. レビューイ（レビューされる側）としての工夫 ・差分のサイズ ・コミットを分割 ・意味のあるまとまり ・コメントで補足 ・必要な情報を揃える etc… 集まった意見はすべて 「いかにレビューしやすくするか」 にフォーカスしたもの
105. レビューは コミュニケーションと 思いやりめっちゃ大事
106. Androidならではのレビュー観点
107. iOSと比較して端末が多種多様（性能も画面サイズも） →パフォーマンス面、UI面の考慮が必要 Androidならではのレビュー観点
108. パフォーマンスについて 　・メインセーフなつくりになっているか 　・メモリリークを起こすところが無いか
 　　（LeakCanaryなどのツールを使う） 　・レイアウトの入れ子構造が深くないか Androidならではのレビュー観点
109. Androidならではのレビュー観点 UIについて 　・画面が小さい端末や、逆に大きい端末での表示が
 　　考慮できているか 　・文字サイズや画面ズームの設定を変更しても崩れないか
110. その他 　・Activity破棄時・再生成時の挙動を考慮できているか 　・OSバージョン間での挙動に問題がないか Androidならではのレビュー観点
111. 〜アジェンダここまで〜
112. セッションの目標の振り返り コードレビューに対して ほんのちょっと 前向きな気持ちに
113. Special Thanks ・インタビューに応じてくださった皆さん ・このスライドをレビューしてくださった皆さん ・セッションを聴いてくださった皆さん ありがとうございました！
114. 最終ページ