CodeReviewerが求められること

CodeReviewer が求められること PHP Conference Japan 2023 おのぽん @onopon_engineer

ビュワー・ビュアー論争をしたくないので、 Reviewerに統一します同様に、ビューイ・ビュイーもRevieweeと統一します 2 はじめに

自己紹介株式会社ウエディングパーク Photorait 本部おのぽん（@onopon_engineer）・2022年6月にウエディングパークに入社・前職では　　- 大規模ソーシャゲームのサーバエンジニア
　　- 人工知能コミュニケーションボットのサーバ全般　　- Androidアプ開発　を行い、業務でのPHP開発は未経験 → 個人での外部登壇も初めてなので、お手柔らかにお願いいたしますmm ・エンジニア以外の時は大体卓球をしています🏓 ・パ卓球選手のコーチもしており、　2024年のパパンピックでの活躍を目指し日々奮闘中 3

Photorait 全国のフォトウエディング・前撮りの撮影スタジオ・サン情報と共にクチコミを掲載している日本最大級のフォトウエディング専門クチコミ情報サイト 4

5 突然ですが。このビューコメント、優秀です。

6 突然ですが。このビューコメント、優秀です。

7 本セッションが終わる頃には、「なぜ優秀なのか」を解明します！突然ですが。このビューコメント、優秀です。

今日からできる！ Revieweeから求められるコードビュー術を身につけよう！ 8 本セッションのゴー

• なぜコードビューを行うのか • Reviewerに必要な心構え • インタビュー調査 • 調査結果の分析 •
分析からわかるRevieweeの気持ち • Revieweeの気持ちを理解したコメント例 • 今日からできる！Revieweeから求められるReview術 • まとめ 9 アジェンダ

• 作ろうとしているものが合っているかを確認するため • コードの質を担保するため • 属人化を防ぐため • 知識共有や認識合わせのため • チーム内のスキ
アップのため 10 なぜコードビューを行うのか

11 例えば下記のコードのビュー依頼が来たら、あなたはどのようなビューをしますか？ class User { public function
drinkable($drink) { return !$this->isChild() || !$drink->containsAlchole(); } }

12 修正したいと思いつつも相手が目上の方だったので、 Revieweeの意見が正しいと判断しLGTMを出す class User { public function drinkable($drink) {
return !$this->isChild() || !$drink->containsAlchole(); } } LGTMです！👍

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 状況を確認しつつ、修正コードの提案を行う class User { public function drinkable($drink) { return
!$this->isChild() || !$drink->containsAlchole(); } } 「子供であり、飲み物がアコーの場合はfalse」で、「それ以外の時は true」であってますか？であれば、下記のように書くのはいかがでしょうか？ if ($this->isChild() && $drink->containsAlchole()) { return false; } return true;

15 Revieweeは自分の部下かつ他のタスクで手が離せない状況。そのため改善点があることだけを伝え、改善内容は相手に任せる class User { public function drinkable($drink) {
return !$this->isChild() || !$drink->containsAlchole(); } } 修正してください。

• ビューの内容や表現方法も千差万別である ◦ ソースコードの理想像や思想は誰しもが持っている • どのようなビューコメントを残すべきかは状況により異なる ◦ チームのみんなが運用できる状況にコーディングを整える ◦
未熟なRevieweeを育てるために議論するコメントを増やす ◦ 今後のためにわからない箇所は質問する 16 コードビューに正解はない

コードビューに正解はないが、不正解はある 17

新卒1年目おのぽん：「ビューお願いします！」 18

新卒1年目おのぽん：「ビューお願いします！」 19

新卒1年目おのぽん：「ビューお願いします！」〜 20秒後〜先輩エンジニア：「コメントしました。」 20

新卒1年目おのぽん：「ビューお願いします！」〜 20秒後〜先輩エンジニア：「コメントしました。」 21

新卒1年目おのぽん：「ビューお願いします！」〜 20秒後〜先輩エンジニア：「コメントしました。」周囲のエンジニア：「www」 22

新卒1年目おのぽん：「ビューお願いします！」〜 20秒後〜先輩エンジニア：「コメントしました。」周囲のエンジニア：「www」 23

新卒1年目おのぽん：「ビューお願いします！」〜 20秒後〜先輩エンジニア：「コメントしました。」周囲のエンジニア：「www」エンジニアは怖い生き物なのだと思い始める 24

• 相手を思いやってビューを行う ◦ 思いやりのないビューはチーム内での関係性の破綻を招いたり、人を傷つけてしまう恐れがある • 思いやるためにはどうすれば良いか ◦
まずはその人（= Reviewee）のことをよく理解しよう 25 Reviewerに必要な心構え

• 対象：20-30代の弊社のいろんな役職のPHPerエンジニア • 人数：10名 • インタビュー時間：10分 • 内容 26 Revieweeの理解を深めるインタビュー
Reviewを依頼する際、 • どのようなビューを求めるか？ • ビューを出す際に意識していることは？ • どんなコメントが残ってると嬉しい？ • こんなビューが助かった • こんなビューは嫌だった

KA法とは、定性調査で明らかにしたターゲットの声や行動・体験などの「質的データ」を分析・モデングし、本質的なニーズやインサイトを明らかにしていく手法 27 インタビュー結果をKA法を用いて潜在ニーズを明らかに参考URL: https://www.unprinted.design/articles/ka-method/#3a3126842b78ec5125e19f33a7956099 インタビューにより得られた回答を 1文ずつ出来事に記載する出来事から考え得る心の声を記載
行動の背景にある価値を記載

28 実際に行ったKA法の様子 100枚を超えるKAカードを作成！

グループ② グループ① KJ法とは、書き出した情報の断片的な要素を関連づけて、俯瞰しながらグープ化していく手法 29 KJ法を用いて潜在ニーズを分析参考URL: https://www.unprinted.design/articles/ka-method/#3a3126842b78ec5125e19f33a7956099

30 実際に行ったKJ法の様子 7つにグーピング！

31 気づきや学びが欲しい楽しく働きたい自信を持ちたい実装や意図を理解して欲しい Reviewerへの心遣い属人化を
防ぎたい利用ユーザに早く価値を届けたい Revieweeのニーズ

防ぎたい利用ユーザに早く価値を届けたい ReviewerがRevieweeの要望に応えることができるもの

33 先ほどのビュー例はRevieweeから求められるビューとなっているのか下記の3点の軸で、ビューコメント例を 100点満点で評価してみる ①気づきや学びを与えることができているか ②実装や意図を理解していることが伝わるか ③Revieweeに自信を持たせることができているか
気づきや学びが欲しい自信を持ちたい実装や意図を理解して欲しい

return !$this->isChild() || !$drink->containsAlchole(); } } LGTMです！👍

return !$this->isChild() || !$drink->containsAlchole(); } } LGTMです！👍 ①気づきや学び × Reviewerの修正したいという想いは届かない ②実装や意図を理解している △ コメントだけでは判断しきれない。・普段LGTMしか書かないReviewerの場合、理解しているかどうかが判断できない・普段コメントの多いReviewerの場合、理解してくれているような気がする ③Revieweeに自信を持たせる △ こちらも②と同様・普段指摘コメントの多いReviewerからのLGTMは、 Revieweeに自信を持たせることができる

return !$this->isChild() || !$drink->containsAlchole(); } } LGTMです！👍 ①気づきや学び × Reviewerの修正したいという想いは届かない ②実装や意図を理解している △ コメントだけでは判断しきれない。・普段LGTMしか書かないReviewerの場合、理解しているかどうかが判断できない・普段コメントの多いReviewerの場合、理解してくれているような気がする ③Revieweeに自信を持たせる △ こちらも②と同様・普段修正コメントの多いReviewerからのLGTMは、 Revieweeに自信を持たせることができる 25点

!$this->isChild() || !$drink->containsAlchole(); } } すみません。このジックの意図を教えていただいてもよろしいでしょうか？（本心：わかりづらいから変えてほしい） ex) return !($this->isChild() && $drink->containsAlchole()); or return $this->isAdult() || $drink->isSoftdrink();

!$this->isChild() || !$drink->containsAlchole(); } } すみません。このジックの意図を教えていただいてもよろしいでしょうか？（本心：わかりづらいから変えてほしい） ex) return !($this->isChild() && $drink->containsAlchole()); or return $this->isAdult() || $drink->isSoftdrink(); ①気づきや学び ◯ このコメントからは得られない。　しかし、本心は変えて欲しいので、　返答コメントに対する返答で学びを与えることが可能。 ②実装や意図を理解している ◯ このコメントからは理解できていないように見える。　こちらもやり取りの中でRevieweeが感じことができる。 ③Revieweeに自信を持たせる × 指摘する流れのコメントとなる可能性が高いので、自信には繋がらない

!$this->isChild() || !$drink->containsAlchole(); } } すみません。このジックの意図を教えていただいてもよろしいでしょうか？（本心：わかりづらいから変えてほしい） ex) return !($this->isChild() && $drink->containsAlchole()); or return $this->isAdult() || $drink->isSoftdrink(); ①気づきや学び ◯ このコメントからは得られない。　しかし、本心は変えて欲しいので、　返答コメントに対する返答で学びを与えることが可能。 ②実装や意図を理解している ◯ このコメントからは理解できていないように見える。　こちらもやり取りの中でRevieweeが感じことができる。 ③Revieweeに自信を持たせる × 指摘する流れのコメントとなる可能性が高いので、自信には繋がらない 80点

!$this->isChild() || !$drink->containsAlchole(); } } 「子供であり、飲み物がアコーの場合はfalse」で、「それ以外の時は true」であってますか？であれば、下記のように書くのはいかがでしょうか？ if ($this->isChild() && $drink->containsAlchole()) { return false; } return true;

!$this->isChild() || !$drink->containsAlchole(); } } 「子供であり、飲み物がアコーの場合はfalse」で、「それ以外の時は true」であってますか？であれば、下記のように書くのはいかがでしょうか？ if ($this->isChild() && $drink->containsAlchole()) { return false; } return true; ①気づきや学び ◯ 別の表現方法を提案しているので、学びにつながる。 ②実装や意図を理解している ◯ 一度確認の文章を入れることで、　　　理解している様子を伝えることができている。 ③Revieweeに自信を持たせる △ 指摘コメントとなってしまっているものの、やっている方向性は合っていることが伺えるので、自信は現状維持くらい。

!$this->isChild() || !$drink->containsAlchole(); } } 「子供であり、飲み物がアコーの場合はfalse」で、「それ以外の時は true」であってますか？であれば、下記のように書くのはいかがでしょうか？ if ($this->isChild() && $drink->containsAlchole()) { return false; } return true; ①気づきや学び ◯ 別の表現方法を提案しているので、学びにつながる。 ②実装や意図を理解している ◯ 一度確認の文章を入れることで、　　　理解している様子を伝えることができている。 ③Revieweeに自信を持たせる △ 指摘コメントとなってしまっているものの、やっている方向性は合っていることが伺えるので、自信は現状維持くらい。 85点

return !$this->isChild() || !$drink->containsAlchole(); } } 修正してください。

return !$this->isChild() || !$drink->containsAlchole(); } } 修正してください。 ①気づきや学び × Revieweeはこの記述が正しいと思っているので、何で修正依頼をされているのかわからないこのコメントからは何も学ぶことができない ②実装や意図を理解している × Reviewerは苦肉の策でこのようなコメントを残している可能性はあるものの、意見を聞き入れる様子がないため、意図を理解しているように見えない ③Revieweeに自信を持たせる × Revieweeは修正の指摘を一方的に受けるため自信は持てない

return !$this->isChild() || !$drink->containsAlchole(); } } 条件がわかりづらいので修正してください。 ①気づきや学び × Revieweeはこの記述が正しいと思っているので、何がわかりづらいのか気づけないこのコメントからは何も学ぶことができない ②実装や意図を理解している × Revieweeは苦肉の策でこのようなコメントを残している可能性はあるものの、意見を聞き入れる様子がないため、意図を理解しているように見えない ③Revieweeに自信を持たせる × Revieweeは修正の指摘を一方的に受けるため自信は持てない 0点

例題 \ 評価軸 ①気づきや学び ②実装や意図を理解している ③Revieweeに自信を持たせる点数１つ目
× △ △ 25 ２つ目 ◯ ◯ × 80 ３つ目 ◯ ◯ △ 85 ４つ目 × × × 0 46 (※1) (※2) 例題の評価一覧 ※1 : 普段指摘コメントの多い人からのLGTMは自信を持たせることができる ※2 : 指摘コメントとなってしまっているものの、やっている方向性は合っていることが伺えるので、自信は現状維持くらい。

× △ △ 20 ２つ目 ◯ ◯ × 80 ３つ目 ◯ ◯ △ 85 ４つ目 × × × 0 47 (※1) (※2) 例題の評価一覧 ※1 : 普段指摘コメントの多い人からのLGTMは自信を持たせることができる ※2 : 指摘コメントとなってしまっているものの、やっている方向性は合っていることが伺えるので、自信は現状維持くらい。気づきや学び、実装や意図を理解していることを伝えることは可能

48 気づきや学び・実装や意図を理解していることを伝えるビュー例実装や意図を理解した上で、ブッシュアップ（学び）のためのコメントを記載物腰柔らかい口調 Revieweeの修正後、感謝を伝える・PHP特有のメソッドや、
ジックの代替案は気づきや学びとして伝えられる・また、「気づきや学びのコメントをする ⇒ 実装や意図を理解している」である（逆は成り立たない）

× △ △ 25 ２つ目 ◯ ◯ × 80 ３つ目 ◯ ◯ △ 85 ４つ目 × × × 0 49 (※1) (※2) 例題の評価一覧 ※1 : 普段指摘コメントの多い人からのLGTMは自信を持たせることができる ※2 : 指摘コメントとなってしまっているものの、やっている方向性は合っていることが伺えるので、自信は現状維持くらい。

× △ △ 25 ２つ目 ◯ ◯ × 80 ３つ目 ◯ ◯ △ 85 ４つ目 × × × 0 50 (※1) (※2) 例題の評価一覧 ※1 : 普段指摘コメントの多い人からのLGTMは自信を持たせることができる → 裏を返すと普段自信を持つことのできないコメントが多い ※2 : 指摘コメントとなってしまっているものの、やっている方向性は合っていることが伺えるので、自信は現状維持くらい → ×よりの△ ビューの中で自信を持たせることは難しい？

51 自信につながるビューって何だろう？

防ぎたい利用ユーザに早く価値を届けたい ReviewerがRevieweeの要望に応えることができるもの心の声にヒントが隠れてそう

53 自信を持ちたいとカテゴイズされた中の意見を見てみる

54 自信を持ちたいとカテゴイズされた中の意見を見てみる

55 安心感が増すビュー例① ・わからない点を質問している →②実装や意図を理解しようとしてくれている・ちゃんとビューをしていないとわからないような質問を投げかけている
納得した上でLGTM →ただLGTMをもらうよりも安心できる

56 安心感が増すビュー例② 読んで理解したことをそのまま書いているだけ。（他の人のReviewerの手助けになる可能性がある）上記のコメントに乗っかっているだけ。ただコメントを残すだけでも Revieweeにとっては安心に繋がるコメントとなり得る

57 安心感が増すビュー例③ ただLTGMを伝えるだけでなく、ビューしていないと書けないようなコメントを添えることで、実装や意図を理解していることを一緒に伝える「LGTMです！」だけでは、「Reviewerが本当に読んだのかどうか」までを伝えることはできず、Revieweeの安心感に繋がらない修正範囲がある程度のファイ
数に及ぶものであれば、 • そのファイの構成 • ジックを読んだからこそ書けるようなコメントを添えるのようなコメントをすることで、Revieweeに安心感を与える

58 褒めるビュー例①② アクションをする（コメント記載者以外の人が　アクションを残していた）コメント + 修正差分を読んで褒めている
→ ②実装や意図を理解しようとしているただ褒めるだけでも良いただ褒めるだけでもRevieweeにとって自信に繋がる肯定的なアクションもGood

59 褒めるビュー例③

60 褒めるビュー例③ ただ感謝の気持ちを伝えるだけでなく、その修正をどれくらい喜んでいるかも一緒に伝える →Revieweeは「修正の方向性は間違えてなかった」と感じる手法やプセスなどを褒める • 感謝の気持ちと共にどれくらい喜んでいるのかを大げさに伝えると自信を持てる
• 手法やプセスを褒めるのもGood

61 本セッションのゴー

今日からできる！ Revieweeから求められるコードビュー術を身につけよう！ 62 本セッションのゴー

63 今日からできる！Revieweeから求められるコードビュー術 Revieweeが求めるビュー ①気づきや学びを与えることができているか ②実装や意図を理解しているように見せられているか ③Revieweeに自信を持たせることができているか　→ 安心したいというインサイト
コードビュー術 • 代替案を伝えるコメントは、①②を満たせる • わからない点は質問することで、②を満たそうとする姿勢を伝えることができる • ジックを読んだからこそ書けるコメントは②を満たすことができ、③にも繋がる • 何かしらを褒めるだけでも③を満たすことができる ◦ それがプセスや手法であっても良い • 肯定的なアクションスタンプは安心につながり、③を促す • 感謝の気持ちを伝える上での大げさな感情表現は③を満たすことができる

64 まとめ • ビューに答えはない • しかし、ビューをする際は前提として思いやりを持つことが大切 • Revieweeの気持ちを知るために、インタビューを行った •
インタビュー結果を分析することで、 RevieweeがReviewerに対して下記3点を求めていることがわかった ◦ 気づきや学びが欲しい ◦ 実装や意図を理解して欲しい ◦ 自信を持ちたい • 本セッションでは、上記を満たすコメント例やビュー術を紹介

本日の内容はブグとして公開しました！ https://engineers.weddingpark.co.jp/code_review_technique/ ウエディングパーク自社イベント wphpcon（LT会）@オンイン開催します！日時：2023年10月27日（金） 19:00 〜（予定）
内容：ウエディングパークのPHPerによる、　　　PHPerのための登壇もできるゆるいLT会詳細はXにて！ @WeddingParkTECH 65 告知です！今日からできる！Revieweeから求められるコードビュー術

66 ご清聴ありがとうございました。素敵なビューイフを！

CodeReviewerが求められること

CodeReviewerが求められること

More Decks by onopon

Other Decks in Programming

Featured

Transcript