並行開発のためのコードレビュー

by Miyuki

Slide 1

Slide 1 text

Slide 2

Slide 2 text

Slide 3

Slide 3 text

Slide 4

Slide 4 text

Slide 5

Slide 5 text

Slide 6

Slide 6 text

Slide 7

Slide 7 text

© NOT A HOTEL, Inc. 7 今日お話しすること - オーナーアプリのWebフロントエンドチームのレビュー体制について - 並行開発を行うための役割分担とレビューの進め方 - レビューをボトルネックにしないための工夫 - アジェンダ - チームの前提 - レビューの目的 - 実装者・レビュアーそれぞれの責任範囲 - レビューのフロー - 実装者・レビュアーがそれぞれ意識すること - コミュニケーションの工夫

Slide 8

Slide 8 text

© NOT A HOTEL, Inc. 8 オーナーアプリチームの前提 - 開発フロー - 1 sprint = 3 week - タイトなスケジュールで多くの新機能・改善をリリース - 3 client 同時開発・同時リリース - 機能ごとに各職種1、2名アサインの横断チームで開発 - キックオフで全員が概要は把握するが、開発に入ると細部の仕様は担当者しか把握していない状態になりやすい - レビューは職種ごとのチームで - QA - リリースする機能はすべてQA - PdMと連携してテストケースを作成 - メンバー - Webのメンバー3人は全員1年以上在籍 - 一人で一つの機能をサポートなしで担当できる機能A 機能B 機能C 機能D

Slide 9

Slide 9 text

Slide 10

Slide 10 text

Slide 11

Slide 11 text

© NOT A HOTEL, Inc. 11 実装者の責任 - 仕様に沿った実装 - 仕様書・Figmaとの突き合わせ - 挙動を担保する - 動作確認をした状態でレビュー依頼 - 確認のキャプチャを画像や動画でPR説明につけたり、確認したパターンを列挙したりする - AIの書いたコードをレビューして仕上げる - 仕様の理解と設計までやってコーディングはAIがすることも増えている - ただし冗長な書き方や変に複雑化させていることも多いので、実装者が検品した上で本人のコードとしてレビューに出す責任の線引き

Slide 12

Slide 12 text

© NOT A HOTEL, Inc. 12 責任の線引きレビュアーの責任 - 実装者が共有した仕様を前提にした品質の担保 - 仕様に対して設計が自然か - 担当者以外もメンテナンスできるか - 将来の変更に耐えられるかやらないこと - すべてのPRの動作確認や、仕様書との網羅的な突合せはしない - 実装者とQAで担保できている - QAで出たバグも基本的に本人が担当するので、実装時から気を抜かない前提 - インタラクションや状態の変化、遷移が複雑な機能などは、手元で挙動も確認する場合もあるが、全部はしない

Slide 13

Slide 13 text

© NOT A HOTEL, Inc. 13 レビューのフロー 1. Pull Requestをopen - テンプレートに沿ってPR説明を記入 2. Claudeの自動レビューを確認し、指摘を修正 - コーディング規約、レビュー観点にそった一次レビュー 3. 行コメントでレビュアー向けの補足・解説 - セルフレビューも兼ねる 4. Slackでレビュー依頼 5. 基本は1 approveでマージ - 仕様共有のためや、複数観点ほしいときは2 approve要求

Slide 14

Slide 14 text

Slide 15

Slide 15 text

© NOT A HOTEL, Inc. 15 実装者が意識すること：レビュアーの負荷を下げる 1. PRを小さくする - 基本は1つのPRには1つの主題 - 新規機能は特に大きくなりがちなので、レビュー観点ごとに分割 - コンポーネントやファイル構成だけ追加するPR（命名やコンポーネントの分け方などを見る） - 全体の処理の流れを完成させるPR（ロジックをしっかり見る） - UIの作り込みをするPR（見た目が確認できていればそこまで指摘はない） - 必要なAPIリクエストだけ追加するPR（決まった書き方なのでほぼ指摘なし） - モック部分を実際のリクエストに差し替えるつなぎこみPR（動作していればほぼ指摘なし） - 逆にテストコードと実装は分けない方が、仕様も明確になり挙動の担保を補強するのでレビューのしやすさにつながる - Jest: ロジックの重要な部分 - Cypress: UIの見た目、表示の分岐パターン

Slide 16

Slide 16 text

© NOT A HOTEL, Inc. 16 2. わかりやすいPR説明 - コードを読む前の前提知識の共有、仕様共有 - issue / 仕様書 / Figma へのリンク - 変更の背景・目的 - リンクすべては情報量が多いので、PRのスコープに絞ってまとめる - 変更内容 - 具体的に何を変更したか - 行コメントのほうが理解できそうな場合はそちらで - スクリーンショット・動画 - 変更前との比較がわかりやすい場合は変更前のものも実装者が意識すること：レビュアーの負荷を下げる

Slide 17

Slide 17 text

Slide 18

Slide 18 text

© NOT A HOTEL, Inc. 18 実装者が意識すること実装者の負荷が高くかえってスピードダウンでは？🤔 - PR分割 - 作業のスコープを分割することはAIコーディングの作業精度も上がり実装者自身も検品しやすい - 1PRの議論ポイントが減るのでマージまでのスピードは速くなる - PR説明 - 実装者が動作確認したパターンを示すことで、レビュアーが二重で確認する手間を省ける - 丁寧な記述は、後からのトラブル調査や仕様確認にも役立つので、長期的なチームでの運用にはプラス - 行コメント - コメントする過程がセルフレビューにもなってコードの完成度も上がり、指摘事項が減ることで速くなる - スラスラ読めるようになっている信頼がレビューへの心理的ハードルを下げ、レビューサイクルが速くなる 👉トータルで見るとレビューサイクルは向上、長期目線でのデリバリースピードも上がる

Slide 19

Slide 19 text

© NOT A HOTEL, Inc. 19 レビュアーが意識すること - 自分の作業よりレビューを優先 - レビュー待ちが長いとチーム全体としては時間のロス - 引き継ぐつもりでコードを理解する - 言及されていない仕様の疑問点などもレビューの中で解消する - いまの文脈を知らないメンバーに引き継いだとしてもメンテしやすいようなコードにする - 意図や経緯が読み取りにくい、誤解を招くような箇所をつぶす - 何回か指摘していることはレビュー観点などに反映して、AIレビューで担保できることを増やす

Slide 20

Slide 20 text

© NOT A HOTEL, Inc. 20 レビュアーとして意識すること重点的に見るところ👀 - 読みにくい箇所を見つけ、なぜ読みにくいのか？を掘り下げる - 責務の分け方が不自然、状態管理が冗長、命名が誤解しやすい、など - バグにつながりそう、仕様変更に弱そうな実装 - 怪しい初期値、型で正しく表現できていない部分、など - 共通化した方がいい箇所、逆にしない方がいい箇所 - 仕様への違和感 - 違和感のある実装や、やけに複雑、不必要に見える条件分岐など、仕様自体が不自然になっている場合もある - 仕様をシンプルに調整できないかも検討し、要件を満たしつつコードに複雑性を持ち込まないようにする - プロダクトとしてあるべき状態や運用面での懸念なども意識して、ときには仕様から調整することも自社開発では重要

Slide 21

Slide 21 text

Slide 22

Slide 22 text

© NOT A HOTEL, Inc. 22 まとめ 👉開発速度とコード品質担保を両立するために - チーム全体の効率を意識して相手の時間を尊重する - 実装者はレビュー負荷を下げるための準備をした上で依頼する - レビュアーは作業よりも優先ですぐ見る - 責任範囲を明確にして効率化 - 実装者は仕様や挙動を担保 - レビュアーは長期的な運用を考慮して設計・品質を担保 - レビューを通して仕様と実装の共有をすることで並行開発や柔軟な担当の入れ替えを可能にしている 👉開発体制、メンバー、AIの状況などでレビューの最適解は変わる - 状況に合わせて、やりかたを常にアップデートしていく