こちらのイベントにオンライン参加しました↓
コードレビューどうしてる? 品質向上と効率化の現場Tips共有会 - connpass
トゥギャッターにもまとまっているそうです↓
コードレビューどうしてる? 品質向上と効率化の現場Tips共有会 #コードレビュー_findy - posfie
参加のきっかけ
QAとしてコードレビューする機会はないものの、今回のイベントにもあるようにコードレビューは品質向上にもつながる、ということで参加してみました。
あとは広く「レビュー」として捉えると、テストケースのレビューとかにも応用できる知見があるかな、という思いもありました。
会の概要
4名の方のトークでした。
良いコードレビューとは
発表者の石井さんは、良いコードレビューを
事業成長を加速させるためのコードレビュー
であり、エンジニア側の言葉に寄せると
変化に対応可能で一貫性のあるソフトウェア設計を守るためのコードレビュー
であると捉えているそうでした。これはとても納得です。
この「良いコードレビュー」を行うために、レビュワー・レビュイー双方の気をつけるべき行動とマインドについてポイントを解説するトークでした。
個人的に良いなと思ったのは、レビュワーは指摘時に「良いと思った点は褒める」ということです。
ちょっと前に『間違いだらけの設計レビュー』を読んだのですが、この本でも
- レビューは対象のドキュメントなりコードの改善に繋げないと意味がない
- そのためには相手が意固地になったりせず、改善につながるような雰囲気や伝え方も大事
というような内容が書かれていました。(と私は解釈しました。)
コードレビューでもコレは同じで、褒めることで他の指摘も改善されやすいという側面もありそう、と聞いてて思いました。
さらにこの「良いと思った点は褒める」というのは、なにもレビュイーの気持ちをアゲようというだけではないようです。良い点を見つけて褒めるのはレビュワーにとっての言語化の訓練にもなる、とおっしゃっていて、これまたなるほどと思ったところでした。
ぼくらのPRレビューフロー改善奮闘記
タイトルの通り、プルリクエストのレビューのやり方を色々と工夫してきた経緯についてのお話でした。
全体通して「なるほど」「わかる」の連続でしたね・・・
最初マネージャーが全プルリクのレビューをしていたものの、ブロッカーになる等して「これは変えねば」と考えたところがスタートだったそうです。そこから、全チームメンバーでのモブレビューに変えたことで滞留は減ったものの、個々のレビュー品質が下がったり「レビュー会で見ればいいや」になったりというデメリットも。そこから非同期レビューなどを経て、現在は「チーム状況に応じたレビュー方法をとろう!」という形になったそうです。
チームによって月間のプルリクの数やメンバー数が違うため、それらに応じたやり方があるよね、という話でした。具体的には、プルリクの数が少なければモブレビューとして皆で同期して行い、プルリクやメンバーが多ければ期限を切って非同期レビューをするなど。
発表聞くだけだと「そらそうやろ」と思う人もいそうですが、自分たちで考えて改善サイクル回すことで本当の意味で組織に浸透するんだろうな・・・という感想です。
AIレビュー導入によるCIツールとの共存と最適化
コードレビューの課題に対してAIレビューを導入して改善をはかった、というお話でした。
当初コードレビューの課題として、
- レビューの質に一貫性がない
- 変更内容のドキュメント作成が負担
- プルリクが大きいと認知負荷が高い
を抱えていたそうです。
AIエージェント(qodo-ai/pr-agent: 🚀 PR-Agent (Qodo Merge open-source): An AI-Powered 🤖 Tool for Automated Pull Request Analysis, Feedback, Suggestions and More! 💻🔍)によるレビューを導入したことで、タイプミスなど初歩的なものをAIに発見させつつ、プルリクのタイトルや変更箇所、説明などを整理・要約させて、それを人間が読んで概要掴んだうえでレビューに入るのが効果的、だったそうです。
課題のうち「変更内容のドキュメント作成が負担」のところに特に効いた、とのことでした。
コードレビューの質の一貫性、についてはAIエージェントはどうしてもドメイン知識が弱いため解決できたとまでは言えない、微妙とのこと。一方プルリクが大きいために認知負荷が高いという点は、プルリクの内容要約などで多少効果があったかも、ということでした。
Q&Aでもあったのですが、PR-AgentはAzureOpenAIのAPIキーで簡単に導入できるそうなので、会社によってはするっと試せそうな点もいいですね。
プルリクレビューを終わらせるためのチーム体制
タイトルの通りチーム体制や進め方の話が中心でした。
特に良かったのは「情報を取りに行け!」という話をしている、という点でした。
たとえばプルリク送ってそのまま相手待ち、ではなくて、ちゃんとリマインドしなきゃいけないよね、というのは本当にそうだなと思いました。プルリク以外もそうですよね。
「レビューを依頼して相手にボールが渡ったらいいのではなくて、相手からボールが返ってこないと自分の仕事は終わりにならない、という自覚を持とう」という話をチームメンバーにもされているそうで、大事なやつだ・・・となりました。
他にも、新入社員やインターンの人が迷わないように研修ドキュメントで「わかる範囲でレビューしてApproveしてね!」等明言されているなど、プルリクまわりの色々が整備されているな・・・という印象でした。こういう愚直な取り組みちゃんとやってる系の話は好きです。
感想
それぞれに色々な工夫のお話をされていて、楽しく聞いていました。また色々なヒントも得られた気がします。
テストのレビューをどうやるか的なイベントもそのうち開かれないかな。
その場でのメモ
良いコードレビューとは
- Speee社
- https://x.com/danimal141
- 今日の話
- 良いコードレビューの定義
- 良いコードレビューをチームで運用するためにレビュワー・レビュイーそれぞれ大事にすべきこと
- あつかわないこと
- 使うツール
- 組織での運用
- などHowの話
- 良いコードレビューとは
- Claudeに聞いてみたよ
- 良いコードレビュー=事業成長を加速させるためのコードレビュー
- なにかしら事業を成長させるためのプロダクト開発チームでコードレビューしている・されている方が対象
- こうあるべきだよね、を話す
- なにかしら事業を成長させるためのプロダクト開発チームでコードレビューしている・されている方が対象
- もう少しエンジニア側の言葉で言うと、「変化に対応可能で一貫性のあるソフトウェア設計を守るためのコードレビュー」である
- 目の前の機能開発の目的、完了条件・勝利条件に対して筋が通っていそうか
- 中長期的に良質な顧客体験を提供し続けることを妨げるリスクにどう対処するか
- 中長期的に開発速度を維持・加速させ続けることを妨げるリスクにどう対処するか
- 余談:コードレビューの今後
- 今回は対人間を想定しているが、今後は対AIも増えるかも
- 皆がAIを部下に持つマネージャーのようになり、自分が書くより指示出しがメインになるかも
- ここからレビュワー・レビュイー双方大事にしたい観点
- レビュワー編
- 気をつけるべきポイント
- 完了条件が明確か
- プロダクトにとってのコア価値に繋がる部分の設計が問題ないか
- 非機能要件は明確になっているか
- 気をつけたいポイントマインド編
- 自分が後でそのコードをメンテする想定で臨む
- 人でなくコードを指摘
- なぜ良くないか、の理由をつけて議論
- 好みは押し付けない、不毛
- 良いと思った点は褒める
- 言語化の訓練にもなる
- レビュイー編
- 気をつけたいポイント
- 完了条件やスコープは明確か
- 数カ月後に自分や他人が読んで意図が理解できそうか
- コードだけで意図が伝わりにくそうな部分はコメントやADR(意思決定ドキュメント)を残せているか
- 気をつけたいポイントマインド編
- なぜそのようにしたかの理由を明確にして回答する
- それが難しいなら、レビュー依頼の前にもっと周囲に頼る必要があるのでは?
- 指摘が理解できない場合は積極的に質問
- まとめ
ぼくらのPRレビューフロー改善奮闘記
- 今日の内容
- PRレビューの流れ
- PRレビューのいち付
- 話さないこと
- PRレビューの中身
- コードレビュー観点
- 試行錯誤の最中なので視聴者側の工夫も教えて
- 開発グループのバックグラウンド
- WHI社はCOMPANYという統合人事システムを自社開発している
- その中の勤怠・プロジェクト管理システムを使っている
- メンバー構成
- チーム1 5名PRは月3~5件
- チーム2 PRは月15~30件
- 最初のPRレビューのやりかた:マネージャーが全チームの全PRをレビュー
- マネージャーがブロッカーになってレビュー滞留したり、メンバーの学習機会を奪ったり
- 別ドメインからマネージャーが来ていたので、知識・スキル不足があって最初遅かった
- マネージャーがひとりでやってるのがいけない、と考えた
- 次:全PRを全チームメンバーでモブレビュー
- 先の課題を伝えたうえで、「ダメだったら戻そう!」と言ってはじめてみた
- メンバーもわりと好意的
- やりかた
- 朝解の最後にモブレビュー時間を設けた
- 良い点
- レビュー滞留がほぼなくなった
- レビュアーとしての視座を皆が持てた
- コーディングルールなどもその場で作ったりできた
- 毎日PR見る時間が強制的に訪れるのは楽!
- イマイチな点
- 個々のレビュー品質が下がりやすくなった
- モブレビュー時に初めてPRの中身を見る、という場面がふえて、表面的な指摘に終始しがち
- 「モブレビューの時間に確認すればいいや」
- モブレビュー時に初めてPRの中身を見る、という場面がふえて、表面的な指摘に終始しがち
- メンバーも「もっとちゃんと見たいです」という声が
- 個々のレビュー品質が下がりやすくなった
- さらに次:全PRを全チームメンバーで非同期レビュー
- やりかた
- チーム単位でレビュワーが各々のタイミングでPRレビュー
- 全レビュワーがApproveしたらマージ
- レビュイーの解説が必要なものなどは朝会でモブレビュー実施
- 良い点
- じっくり見られるようになった
- イマイチな点
- 再度レビュー遅延が発生した
- じっくり見る、という名目で後回しにも
- レビュイー側のマルチタスクが頻発した
- レビュー待ってるから他の開発に着手→戻ってきたらいろいろ並行
- メンバーからも「マージが遅れるのしんどい」という声が
- やりかた
- 現在:
- チーム1はモブレビューに回帰
- PR数も少ないので
- チーム2は非同期レビュー
- 期限設定など
- チーム1はモブレビューに回帰
- まとめ
- 至高のレビューフローは存在しない
- いい塩梅のレビューフローを考えよう
- チームでPRレビューに求めるポイントを考えておこう
- チームをとりまく状況も加味しよう
- 至高のレビューフローは存在しない
AIレビュー導入によるCIツールとの共存と最適化
- ナカ「シ」マさん
- コードレビューの課題
- レビューの質に一貫性がない
- 変更内容のドキュメント作成が負担
- PRが大きいと認知不可が高い
- なのでAIレビューの導入を検討
- AIレビュー(PR-Agent)導入の目的
- レビューコスト削減とコード品質向上
- 初回レビューの自動化
- タイプミスなど初歩的なものの見落としを未然に防ぐ
- PRタイトルや説明を整理してレビュアーの理解促進
- 初回レビューの自動化
- レビューコスト削減とコード品質向上
- PR-Agentの概要
- 公式リポジトリ参照
- オープンソースのAIツール
- Azure OpenAI API Keyがあると簡単に導入できる
- 導入で気付いたこと
- メリット
- よくあるミスや言語仕様の理解度が高い
- PRの整理・要約がいい
- デメリット
- ドメイン知識が弱い
- ベストプラクティスの適用判断が難しい
- あえて実施していないものなど適切に判断できない
- メリット
- コードレビューの課題
- レビューの質に関してはAIエージェントでは解決できたか微妙
- 変更内容のドキュメント作成の負担はある程度効果があった
- PRが大きいと認知負荷が高い件は、指摘に関しては微妙。上のドキュメンテーションで多少効果があったかな
- 既存のCIツール(Linterなど)とどう違うのか
- AIレビューは対応を人が判断する
- まとめ
- AIレビュー
- 標準的な指摘はできるが、最終的には人の判断が要る
- AIツールとCIツール棲み分け
- CIツールは機械的にPass/Fail決める部分を担当
- AIツールは人間のレビューの補助
- AIレビュー
- Q:レビューポイントをまとめてくれる、というのはどういうものが出力されるのか
- 各ファイルごとに「このような修正があった」と示してくれる
- Q:月にいくらくらいかかるようになったのか気になります!
- AzureOpenAPIを使っている
- チームによってPR量が変わってくるので一概には言えないが、自社で複数チームあわせて数千円、1万円かからないくらい。わりと安価。
プルリクレビューを終わらせるためのチーム体制
- yokoe24
- ホンマごめん
- コーディングルール
- 決める必要のある方針
- Linterのどの設定をオンにするか
- Linterにない決め事
- 決める必要のある方針
- プルリクテンプレ
- ラブグラフ社のものはzennにアップしてある
- レビュー周りのルール
- 新入社員やインターン、自分がレビューしていいのか迷いがち
- 研修ドキュメントで明言する
- 複雑な議論が起きそうなときにSlackでのやりとりを推奨するルールも、レビューを早めるために大事かもしれない
- 新入社員やインターン、自分がレビューしていいのか迷いがち
- 情報をとりにいく
- 情報はpullよりpush
- 情報を取りに行く人は少ない、という大前提
- レビューをすることはいいこと、を伝える。一方で、リマインドもしないのにレビューが必ずされると思ってはいけないということも伝える。
- レビュー依頼したとて、相手からボールが返ってこないと自分のしごとはおわらない、という自覚を持つ
- Slackへ自動リマインド
- インターンはリマインドにも限界があるだろう、ということでOSS作った
- 時間をつくることの大事さ
- それでもやっぱりPRは溜まってしまう。これは自然の摂理。
- みんなでレビューする時間をつくろう。
- レビュー会の試み
- 週に3回、30分
- PR作成者が説明したあとで、他全員がレビューする
- メリデメ
- レビュー負荷が下がった
- 一方で「レビュー会あるからそのときに」ということで後回しにする人が発生するかも?