CodeReviewerが求められること

https://fortee.jp/phpcon-2023/proposal/25891e6c-7762-47b5-8cb3-e3db7f056abc
誰かの書いたコードが世に出る過程で、コードレビューを行う組織は多いのではないでしょうか。
コードの質を担保するため。属人化を防ぐため。知識共有や認識合わせのため。チーム内のスキルアップのため。

コードレビューは様々な意味や目的を持って行われます。

本セッションでは、コードレビュワーがコードレビューイに求められていることとそれに応える方法を下記の例と共に考察していきます。
・PHPコード上でのレビュワーとレビューイのコミュニケーション例
・レビューが活性化するPHPコード例

また、実際に弊社で働く様々なポジションのPHPerエンジニアにインタビューを行い、彼らが求めるレビュー内容をもとに共通項を見つけたり、
レビュワーとしてコメントをするときの工夫点や注意点を紹介します。

レビュー時、依頼時に少しでもお役に立てると幸いです。

対象者： コードレビューを行う全エンジニア（役職問わず）

1. CodeReviewer が求められること PHP Conference Japan 2023 おのぽん @onopon_engineer
2. ビュワー・ ビュアー論争をしたくないので、 Reviewerに統一します 同様に、 ビューイ・ ビュイーもRevieweeと統一します 2 はじめに
3. 自己紹介 株式会社ウエディングパーク Photorait 本部 おのぽん（@onopon_engineer） ・2022年6月にウエディングパークに入社 ・前職では 　　- 大規模ソーシャ ゲームのサーバエンジニア 　　- 人工知能コミュニケーション ボットのサーバ全般 　　- Androidアプ 開発 　を行い、業務でのPHP開発は未経験 → 個人での外部登壇も初めてなので、 お手柔らかにお願いいたしますmm ・エンジニア以外の時は大体卓球をしています🏓 ・パ 卓球選手のコーチもしており、 　2024年のパ パ ンピックでの活躍を目指し日々奮闘中 3
4. Photorait 全国のフォトウエディング・前撮りの撮影スタジオ・サ ン情報と共にクチコミを掲載している 日本最大級のフォトウエディング専門クチコミ情報サイト 4
5. 5 突然ですが。 この ビューコメント、優秀です。
6. 6 突然ですが。 この ビューコメント、優秀です。
7. 7 本セッションが終わる頃には、「なぜ優秀なのか」を解明します！ 突然ですが。 この ビューコメント、優秀です。
8. 今日からできる！ Revieweeから求められるコード ビュー術を身につけよう！ 8 本セッションのゴー
9. • なぜコード ビューを行うのか • Reviewerに必要な心構え • インタビュー調査 • 調査結果の分析 • 分析からわかるRevieweeの気持ち • Revieweeの気持ちを理解したコメント例 • 今日からできる！Revieweeから求められるReview術 • まとめ 9 アジェンダ
10. • 作ろうとしているものが合っているかを確認するため • コードの質を担保するため • 属人化を防ぐため • 知識共有や認識合わせのため • チーム内のスキ アップのため 10 なぜコード ビューを行うのか
11. 11 例えば下記のコードの ビュー依頼が来たら、あなたはどのような ビュー をしますか？ class User { public function drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } }
12. 12 修正したいと思いつつも相手が目上の方だったので、 Revieweeの意見が正しいと判断しLGTMを出す class User { public function drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } } LGTMです！👍
13. 13 相手が目上の方だったので、様子を伺う class User { public function drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } } すみません。この ジックの意図を教えていただいてもよろしいでしょう か？（本心：わかりづらいから変えてほしい） ex) return !($this->isChild() && $drink->containsAlchole()); or return $this->isAdult() || $drink->isSoftdrink();
14. 14 状況を確認しつつ、修正コードの提案を行う class User { public function drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } } 「子供であり、飲み物がア コー の場合はfalse」で、「それ以外の時は true」であってますか？ であれば、下記のように書くのはいかがでしょうか？ if ($this->isChild() && $drink->containsAlchole()) { return false; } return true;
15. 15 Revieweeは自分の部下かつ他のタスクで手が離せない状況。 そのため改善点があることだけを伝え、改善内容は相手に任せる class User { public function drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } } 修正してください。
16. • ビューの内容や表現方法も千差万別である ◦ ソースコードの理想像や思想は誰しもが持っている • どのような ビューコメントを残すべきかは状況により異なる ◦ チームのみんなが運用できる状況にコーディングを整える ◦ 未熟なRevieweeを育てるために議論するコメントを増やす ◦ 今後のためにわからない箇所は質問する 16 コード ビューに正解はない
17. コード ビューに正解はないが、不正解はある 17
18. 新卒1年目おのぽん：「 ビューお願いします！」 18
19. 新卒1年目おのぽん：「 ビューお願いします！」 19
20. 新卒1年目おのぽん：「 ビューお願いします！」 〜 20秒後 〜 先輩エンジニア：「コメントしました。」 20
21. 新卒1年目おのぽん：「 ビューお願いします！」 〜 20秒後 〜 先輩エンジニア：「コメントしました。」 21
22. 新卒1年目おのぽん：「 ビューお願いします！」 〜 20秒後 〜 先輩エンジニア：「コメントしました。」 周囲のエンジニア：「www」 22
23. 新卒1年目おのぽん：「 ビューお願いします！」 〜 20秒後 〜 先輩エンジニア：「コメントしました。」 周囲のエンジニア：「www」 23
24. 新卒1年目おのぽん：「 ビューお願いします！」 〜 20秒後 〜 先輩エンジニア：「コメントしました。」 周囲のエンジニア：「www」 エンジニアは怖い生き物なのだと思い始める 24
25. • 相手を思いやって ビューを行う ◦ 思いやりのない ビューはチーム内での関係性の破綻を招いたり、 人を傷つけてしまう恐れがある • 思いやるためにはどうすれば良いか ◦ まずはその人（= Reviewee）のことをよく理解しよう 25 Reviewerに必要な心構え
26. • 対象：20-30代の弊社のいろんな役職のPHPerエンジニア • 人数：10名 • インタビュー時間：10分 • 内容 26 Revieweeの理解を深めるインタビュー Reviewを依頼する際、 • どのような ビューを求めるか？ • ビューを出す際に意識していることは？ • どんなコメントが残ってると嬉しい？ • こんな ビューが助かった • こんな ビューは嫌だった
27. KA法とは、定性調査で明らかにしたターゲットの声や行動・体験などの「質的データ」 を分析・モデ ングし、本質的なニーズやインサイトを明らかにしていく手法 27 インタビュー結果をKA法を用いて潜在ニーズを明らかに 参考URL: https://www.unprinted.design/articles/ka-method/#3a3126842b78ec5125e19f33a7956099 インタビューにより得られた回答を 1文ずつ出来事に記載する 出来事から考え得る心の声を記載 行動の背景にある価値を記載
28. 28 実際に行ったKA法の様子 100枚を超えるKAカードを作成！
29. グループ② グループ① KJ法とは、書き出した情報の断片的な要素を関連づけて、 俯瞰しながらグ ープ化していく手法 29 KJ法を用いて潜在ニーズを分析 参考URL: https://www.unprinted.design/articles/ka-method/#3a3126842b78ec5125e19f33a7956099
30. 30 実際に行ったKJ法の様子 7つにグ ーピング！
31. 31 気づきや学びが欲しい 楽しく 働きたい 自信を持ちたい 実装や意図を 理解して欲しい Reviewerへの 心遣い 属人化を 防ぎたい 利用ユーザに 早く価値を届けたい Revieweeのニーズ
32. 32 気づきや学びが欲しい 楽しく 働きたい 自信を持ちたい 実装や意図を 理解して欲しい Reviewerへの 心遣い 属人化を 防ぎたい 利用ユーザに 早く価値を届けたい ReviewerがRevieweeの要望に応えることができるもの
33. 33 先ほどの ビュー例はRevieweeから求められる ビューとなっているのか 下記の3点の軸で、 ビューコメント例を 100点満点で評価してみる ①気づきや学びを与えることができているか ②実装や意図を理解していることが伝わるか ③Revieweeに自信を持たせることができているか 気づきや学びが欲しい 自信を持ちたい 実装や意図を 理解して欲しい
34. 34 修正したいと思いつつも相手が目上の方だったので、 Revieweeの意見が正しいと判断しLGTMを出す class User { public function drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } } LGTMです！👍
35. 35 修正したいと思いつつも相手が目上の方だったので、 Revieweeの意見が正しいと判断しLGTMを出す class User { public function drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } } LGTMです！👍 ①気づきや学び × Reviewerの修正したいという想いは届かない ②実装や意図を理解している △ コメントだけでは判断しきれない。 ・普段LGTMしか書かないReviewerの場合、 理解しているかどうかが判断できない ・普段コメントの多いReviewerの場合、 理解してくれているような気がする ③Revieweeに自信を持たせる △ こちらも②と同様 ・普段指摘コメントの多いReviewerからのLGTMは、 Revieweeに自信を持たせることができる
36. 36 修正したいと思いつつも相手が目上の方だったので、 Revieweeの意見が正しいと判断しLGTMを出す class User { public function drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } } LGTMです！👍 ①気づきや学び × Reviewerの修正したいという想いは届かない ②実装や意図を理解している △ コメントだけでは判断しきれない。 ・普段LGTMしか書かないReviewerの場合、 理解しているかどうかが判断できない ・普段コメントの多いReviewerの場合、 理解してくれているような気がする ③Revieweeに自信を持たせる △ こちらも②と同様 ・普段修正コメントの多いReviewerからのLGTMは、 Revieweeに自信を持たせることができる 25点
37. 37 相手が目上の方だったので、様子を伺う class User { public function drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } } すみません。この ジックの意図を教えていただいてもよろしいでしょう か？（本心：わかりづらいから変えてほしい） ex) return !($this->isChild() && $drink->containsAlchole()); or return $this->isAdult() || $drink->isSoftdrink();
38. 38 相手が目上の方だったので、様子を伺う class User { public function drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } } すみません。この ジックの意図を教えていただいてもよろしいでしょう か？（本心：わかりづらいから変えてほしい） ex) return !($this->isChild() && $drink->containsAlchole()); or return $this->isAdult() || $drink->isSoftdrink(); ①気づきや学び ◯ このコメントからは得られない。 　 しかし、本心は変えて欲しいので、 　 返答コメントに対する返答で学びを与えることが可能。 ②実装や意図を理解している ◯ このコメントからは理解できていないように見える。 　 こちらもやり取りの中でRevieweeが感じことができる。 ③Revieweeに自信を持たせる × 指摘する流れのコメントとなる可能性が高いので、自信には繋がら ない
39. 39 相手が目上の方だったので、様子を伺う class User { public function drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } } すみません。この ジックの意図を教えていただいてもよろしいでしょう か？（本心：わかりづらいから変えてほしい） ex) return !($this->isChild() && $drink->containsAlchole()); or return $this->isAdult() || $drink->isSoftdrink(); ①気づきや学び ◯ このコメントからは得られない。 　 しかし、本心は変えて欲しいので、 　 返答コメントに対する返答で学びを与えることが可能。 ②実装や意図を理解している ◯ このコメントからは理解できていないように見える。 　 こちらもやり取りの中でRevieweeが感じことができる。 ③Revieweeに自信を持たせる × 指摘する流れのコメントとなる可能性が高いので、自信には繋がら ない 80点
40. 40 状況を確認しつつ、修正コードの提案を行う class User { public function drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } } 「子供であり、飲み物がア コー の場合はfalse」で、「それ以外の時は true」であってますか？ であれば、下記のように書くのはいかがでしょうか？ if ($this->isChild() && $drink->containsAlchole()) { return false; } return true;
41. 41 状況を確認しつつ、修正コードの提案を行う class User { public function drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } } 「子供であり、飲み物がア コー の場合はfalse」で、「それ以外の時は true」であってますか？ であれば、下記のように書くのはいかがでしょうか？ if ($this->isChild() && $drink->containsAlchole()) { return false; } return true; ①気づきや学び ◯ 別の表現方法を提案しているので、学びにつながる。 ②実装や意図を理解している ◯ 一度確認の文章を入れることで、 　　　理解している様子を伝えることができている。 ③Revieweeに自信を持たせる △ 指摘コメントとなってしまっているものの、 やっている方向性は合っていることが伺えるので、自信は現状維持くらい。
42. 42 状況を確認しつつ、修正コードの提案を行う class User { public function drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } } 「子供であり、飲み物がア コー の場合はfalse」で、「それ以外の時は true」であってますか？ であれば、下記のように書くのはいかがでしょうか？ if ($this->isChild() && $drink->containsAlchole()) { return false; } return true; ①気づきや学び ◯ 別の表現方法を提案しているので、学びにつながる。 ②実装や意図を理解している ◯ 一度確認の文章を入れることで、 　　　理解している様子を伝えることができている。 ③Revieweeに自信を持たせる △ 指摘コメントとなってしまっているものの、 やっている方向性は合っていることが伺えるので、自信は現状維持くらい。 85点
43. 43 Revieweeは自分の部下かつ他のタスクで手が離せない状況。 そのため改善点があることだけを伝え、改善内容は相手に任せる class User { public function drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } } 修正してください。
44. 44 Revieweeは自分の部下かつ他のタスクで手が離せない状況。 そのため改善点があることだけを伝え、改善内容は相手に任せる class User { public function drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } } 修正してください。 ①気づきや学び × Revieweeはこの記述が正しいと思っているので、何で修正依頼を されているのかわからない このコメントからは何も学ぶことができない ②実装や意図を理解している × Reviewerは苦肉の策でこのようなコメントを残している 可能性はあるものの、意見を聞き入れる様子がないため、 意図を理解しているように見えない ③Revieweeに自信を持たせる × Revieweeは修正の指摘を一方的に受けるため自信は持てない
45. 45 Revieweeは自分の部下かつ他のタスクで手が離せない状況。 そのため改善点があることだけを伝え、改善内容は相手に任せる class User { public function drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } } 条件がわかりづらいので修正してください。 ①気づきや学び × Revieweeはこの記述が正しいと思っているので、何がわかりづら いのか気づけない このコメントからは何も学ぶことができない ②実装や意図を理解している × Revieweeは苦肉の策でこのようなコメントを残している 可能性はあるものの、意見を聞き入れる様子がないため、 意図を理解しているように見えない ③Revieweeに自信を持たせる × Revieweeは修正の指摘を一方的に受けるため自信は持てない 0点
46. 例題 \ 評価軸 ①気づきや学び ②実装や意図を 理解している ③Revieweeに 自信を持たせる 点数 １つ目 × △ △ 25 ２つ目 ◯ ◯ × 80 ３つ目 ◯ ◯ △ 85 ４つ目 × × × 0 46 (※1) (※2) 例題の評価一覧 ※1 : 普段指摘コメントの多い人からのLGTMは自信を持たせることができる ※2 : 指摘コメントとなってしまっているものの、やっている方向性は合っていることが伺えるので、 自信は現状維持くらい。
47. 例題 \ 評価軸 ①気づきや学び ②実装や意図を 理解している ③Revieweeに 自信を持たせる 点数 １つ目 × △ △ 20 ２つ目 ◯ ◯ × 80 ３つ目 ◯ ◯ △ 85 ４つ目 × × × 0 47 (※1) (※2) 例題の評価一覧 ※1 : 普段指摘コメントの多い人からのLGTMは自信を持たせることができる ※2 : 指摘コメントとなってしまっているものの、やっている方向性は合っていることが伺えるので、 自信は現状維持くらい。 気づきや学び、実装や意図を理解 していることを伝えることは可能
48. 48 気づきや学び・実装や意図を理解していることを伝える ビュー例 実装や意図を理解した上で、 ブ ッシュアップ（学び）のための コメントを記載 物腰柔らかい口調 Revieweeの修正後、感謝を伝える ・PHP特有のメソッドや、 ジックの代替案は気づきや学びとして伝えられる ・また、「気づきや学びのコメントをする ⇒ 実装や意図を理解している」である （逆は成り立たない）
49. 例題 \ 評価軸 ①気づきや学び ②実装や意図を 理解している ③Revieweeに 自信を持たせる 点数 １つ目 × △ △ 25 ２つ目 ◯ ◯ × 80 ３つ目 ◯ ◯ △ 85 ４つ目 × × × 0 49 (※1) (※2) 例題の評価一覧 ※1 : 普段指摘コメントの多い人からのLGTMは自信を持たせることができる ※2 : 指摘コメントとなってしまっているものの、やっている方向性は合っていることが伺えるので、 自信は現状維持くらい。
50. 例題 \ 評価軸 ①気づきや学び ②実装や意図を 理解している ③Revieweeに 自信を持たせる 点数 １つ目 × △ △ 25 ２つ目 ◯ ◯ × 80 ３つ目 ◯ ◯ △ 85 ４つ目 × × × 0 50 (※1) (※2) 例題の評価一覧 ※1 : 普段指摘コメントの多い人からのLGTMは自信を持たせることができる → 裏を返すと普段自信を持つことのできないコメントが多い ※2 : 指摘コメントとなってしまっているものの、やっている方向性は合っていることが伺えるので、 自信は現状維持くらい → ×よりの△ ビューの中で自信を 持たせることは難しい？
51. 51 自信につながる ビューって何だろう？
52. 52 気づきや学びが欲しい 楽しく 働きたい 自信を持ちたい 実装や意図を 理解して欲しい Reviewerへの 心遣い 属人化を 防ぎたい 利用ユーザに 早く価値を届けたい ReviewerがRevieweeの要望に応えることができるもの 心の声にヒントが隠れてそう
53. 53 自信を持ちたいとカテゴ イズされた中の意見を見てみる
54. 54 自信を持ちたいとカテゴ イズされた中の意見を見てみる
55. 55 安心感が増す ビュー例① ・わからない点を質問している →②実装や意図を理解しようとし てくれている ・ちゃんと ビューをしていない とわからないような質問を投げか けている 納得した上でLGTM →ただLGTMをもらうよりも安心 できる
56. 56 安心感が増す ビュー例② 読んで理解したことをそのまま書いているだけ。 （他の人のReviewerの手助けになる可能性がある） 上記のコメントに乗っかっているだけ。 ただコメントを残すだけでも Revieweeにとっては安心に繋がるコメントとなり得る
57. 57 安心感が増す ビュー例③ ただLTGMを伝えるだけでなく、 ビューしていない と書けないようなコメントを添えることで、実装や意図 を理解していることを一緒に伝える 「LGTMです！」だけでは、「Reviewerが本当に読んだのかどうか」まで を伝えることはできず、Revieweeの安心感に繋がらない 修正範囲がある程度のファイ 数に及ぶものであれば、 • そのファイ の構成 • ジックを読んだからこそ書けるようなコメントを添える のようなコメントをすることで、Revieweeに安心感を与える
58. 58 褒める ビュー例①② アクションをする （コメント記載者以外の人が 　 アクションを残していた） コメント + 修正差分を読んで褒めている → ②実装や意図を理解しようとしている ただ褒めるだけでも良い ただ褒めるだけでもRevieweeにとって自信に繋がる 肯定的な アクションもGood
59. 59 褒める ビュー例③
60. 60 褒める ビュー例③ ただ感謝の気持ちを伝えるだけでなく、その修正をどれくらい 喜んでいるかも一緒に伝える →Revieweeは「修正の方向性は間違えてなかった」と感じる 手法やプ セスなどを褒める • 感謝の気持ちと共にどれくらい喜んでいるのかを大げさに伝えると自信を持てる • 手法やプ セスを褒めるのもGood
61. 61 本セッションのゴー
62. 今日からできる！ Revieweeから求められるコード ビュー術を身につけよう！ 62 本セッションのゴー
63. 63 今日からできる！Revieweeから求められるコード ビュー術 Revieweeが求める ビュー ①気づきや学びを与えることができているか ②実装や意図を理解しているように見せられているか ③Revieweeに自信を持たせることができているか 　→ 安心したいというインサイト コード ビュー術 • 代替案を伝えるコメントは、①②を満たせる • わからない点は質問することで、②を満たそうとする姿勢を伝えることができる • ジックを読んだからこそ書けるコメントは②を満たすことができ、③にも繋がる • 何かしらを褒めるだけでも③を満たすことができる ◦ それがプ セスや手法であっても良い • 肯定的な アクションスタンプは安心につながり、③を促す • 感謝の気持ちを伝える上での大げさな感情表現は③を満たすことができる
64. 64 まとめ • ビューに答えはない • しかし、 ビューをする際は前提として思いやりを持つことが大切 • Revieweeの気持ちを知るために、インタビューを行った • インタビュー結果を分析することで、 RevieweeがReviewerに対して下記3点を求めていることがわかった ◦ 気づきや学びが欲しい ◦ 実装や意図を理解して欲しい ◦ 自信を持ちたい • 本セッションでは、上記を満たすコメント例や ビュー術を紹介
65. 本日の内容はブ グとして公開しました！ https://engineers.weddingpark.co.jp/code_review_technique/ ウエディングパーク自社イベント wphpcon（LT会）@オン イン 開催します！ 日時：2023年10月27日（金） 19:00 〜（予定） 内容：ウエディングパークのPHPerによる、 　　　PHPerのための登壇もできるゆるいLT会 詳細はXにて！ @WeddingParkTECH 65 告知です！ 今日からできる！Revieweeから求められるコード ビュー術
66. 66 ご清聴ありがとうございました。 素敵な ビュー イフを！