コードレビューの基本 その1：コードレビューの目的を整理するの続きです。
プルリクエストレビューって、結局どうやって進めるの？
プルリクエスト（以下PR）レビューは、ソフトウェア開発においてコードの品質を一定に保つための重要なプロセスです。PRでは、担当者がコードの変更点をチームメンバーと共有し、チームメンバーはその変更点に対してフィードバックを行います。担当者は、その内容を受けてバグの早期発見やロジック上の課題、要件とのズレを確認できます。
システム上のメリットとしては、そうした問題のあるコードをプロダクトに取り込むのを防止し、一定した品質のあるコード・メンテナンス性の高いコードを取り込めます。技術的負債の蓄積を防止する上でも、PRレビューは非常に重要です。
しかし、初めてPRレビューを行うチームでは、その流れや役割分担が不明瞭なことも多いでしょう。本記事では、PRレビューの基本的な流れと役割分担について解説します。
PRレビューの一般的な流れ
PRレビューは、一般的には以下のようなステップで進行します。
- PR作成
- レビュー依頼
- コメントと修正
- 承認とマージ
PR作成
開発担当者がコードの変更を行い、その内容をまとめたものとしてPRを作成します。GitHubやGitLabなどのプラットフォームを使っている場合、ブランチを切り、変更内容をコミットしてプルリクエストを作成します。この際、PRのタイトルや説明文に、変更の目的や内容を明確に記載することが重要です。
GitHubでは .github/pull_request_template.md というテンプレートを利用できます。このファイルはPR作成時に自動的に読み込まれ、PRの内容を記入するためのガイドラインとなります。テンプレートには、変更点の概要、テストの有無、関連する課題番号などを記載する項目を設けると良いでしょう。
また、コミットメッセージの先頭に記述した方がいいキーワード（例：fix:, feat:, docs:など）を利用し、変更の種類を明確にすることも推奨されます。
【Git】コミットメッセージの先頭につけた方が良い単語リスト- prefix集 - #GitHub - Qiita
レビュー依頼
PRを作成したら、チームメンバーにレビューを依頼します。GitHubやGitLabでは、PRの画面からレビュアーを指定することができます。設定によって、レビューを必須にし、必ず誰かしらの目を通すような運用も可能です。
GitHubでは、複数の方法でレビュアーの自動アサインができます。
Teamのコードレビュー設定の管理 - GitHub Docs
1つはコードオーナーで、 .github/CODEOWNERS または docs/CODEOWNERS というファイルを作成します。このファイルの中で、特定のパスに対して自動的にレビュアーを指定できます。例えば、以下のように記述します。
# .github/CODEOWNERS
# 特定のディレクトリに対して自動的にレビュアーを指定
/docs/ @your-username
/src/ @your-team
もう一つは、ラウンドロビン方式での自動アサインです。GitHubの設定で、特定のチームメンバーに順番にレビューを依頼できます。これにより、特定の人に負担が集中することを防ぎます。
Teamのコードレビュー設定の管理 - GitHub Docs
この時のアルゴリズムは2パターン用意されています。
- ラウンドロビン方式
レビュアーを順番に割り当てる - ロードバランス方式
レビュアーの負荷を均等に分散する
コメントと修正
PRにアサインされたレビュアーは、コードの変更点を確認し、必要に応じてコメントを残します。コメントには、コードの改善点やバグの指摘、要件とのズレなどが含まれます。レビュアーは、コードの可読性や保守性、テストの有無なども必要に応じて確認します。
開発担当者は、レビュアーからのコメントを受けて、必要な修正を行なったり、追加でコメントしたりします。修正した場合にはPRに追加コミットし、レビュアーに再レビューを依頼します。このプロセスは、コードが承認されるまで繰り返されます。
承認とマージ
PRがレビュアーから承認されると、コードの変更がメインブランチにマージされます。マージ前には、CI/CDパイプラインを通じて自動テストが実行され、コードの品質が保たれていることを最終確認します。
マージ後は、PRをクローズし、関連する課題やチケットを更新します。これにより、コードの変更がどのようにプロジェクトに影響を与えたかを追跡できます。作成したブランチは、不要であれば削除します。
誰がレビューする？役割分担と合意形成
PRレビューにおいて、よくある課題が「誰がレビューするのか？」です。特に、チームメンバーが多い場合や、プロジェクトが複雑な場合、レビュアーの選定は重要なポイントとなります。
チーム規模によるレビュー体制の違い
少人数チームの場合
少人数のチームでは、全員がPRレビューに参加することが一般的です。各メンバーが他のメンバーのコードをレビューすることで、コードの品質を保つとともに、チーム全体の知識共有が促進されます。
少人数の場合、技術領域が分かれてしまうケースが多いでしょう。たとえば、フロントエンドとバックエンドで専門性が異なる場合です。しかし、その場合でも領域を超えてレビューを行うことが推奨されます。これにより、コードの一貫性や全体的なアーキテクチャの理解が深まります。
開発チーム全体が小規模な場合、レビューがCTOやシニアエンジニアなどに偏ってしまい、レビュー負荷が集中してしまうという課題があります。これを防ぐためには、以下のような方法が考えられます。
- レビューの強制ローテーション
レビュー担当者を強制的にローテーションします。負担の分散とともに「誰かがレビューするだろう」という依存を防ぎます。また、全員にレビューの経験を積ませることで、チーム全体のスキル向上にもつながります。 - レビューのペアリング
レビューをペアで行うことで、1人の負担を軽減しつつ、コード理解を深められる期待ができます。ペアレビューは、育成もかねることが多いので、新人とシニアメンバーのペアリングが特に有効です。
中規模チームの場合
5名〜10名くらいの中規模チームでは、レビュアーの選定がより重要になります。以下のような方法でレビュアーを選定することが一般的です。
- 同じチーム内のメンバー
同じ技術領域のメンバーがレビューを行うことで、専門的な知識を活かしたフィードバックが得られます。 - シニアメンバーやリーダー
シニアメンバーやリーダーがレビューを行うことで、コードの品質を高めるとともに、若手メンバーの育成にもつながります。 - ペアレビュー
ペアレビューは、2人の開発者が一緒にコードをレビューする方法です。これにより、リアルタイムでのフィードバックが可能となり、コードの理解が深まります。 - 他チームのメンバー
他のチームのメンバーがレビューは、異なる視点からのフィードバックが得るのに有用です。特に、アーキテクチャや設計に関するレビューで有効です。
大規模チームの場合
大規模なチームでは、レビュアーの選定がさらに複雑になります。以下のようなポイントを考慮することが重要です。
- レビュアーの選定基準
- 技術領域や専門性に基づいてレビュアーを選定します。たとえば、フロントエンドの変更にはフロントエンドの専門家がレビューを行うなど、専任者が担当するケースが多いです。
- レビュアーと承認者の区分
- レビューを行うレビュアーと、最終的な承認を行う承認者を区別します。レビュアーはコードの品質や可読性を確認し、承認者は最終的なマージの判断を行います。
- 専任チームの設置
- 大規模なプロジェクトでは、専任のレビューチームを設置することもあります。このチームは、コードレビューの専門家として、全てのPRをレビューし、品質を保つ役割を担います。言語やパフォーマンス、セキュリティなど、特定の領域に特化したレビューチームを設けることもあります。
レビューの合意形成
PRレビューでは、レビュアーと開発担当者の間で合意形成が重要です。合意形成ができていないと、コードの品質が低下したり、チーム内でのコミュニケーションが悪化したりする可能性があります。合意形成のためには、以下のポイントを考慮することが重要です。
- レビューの目的を明確にする
- コメントの内容を具体的にする
- フィードバックを受け入れる姿勢を持つ
- 合意形成のためのディスカッションを行う
レビューの目的を明確にする
PRレビューの目的を明確にすることで、レビュアーと開発担当者の間での認識のズレを防ぎます。レビューの目的は、コードの品質向上やバグの早期発見、要件との整合性確認などです。これは組織やプロジェクトによって異なるため、事前に合意しておかなければなりません。
規模が大きくなった場合には、レビューガイドラインなどの作成が効果的です。
コメントの内容を具体的にする
レビュアーは、レビューガイドラインに沿って、具体的にコメントを記述しましょう。抽象的なコメントはコミュニケーション回数を増やし、PRの承認を遅らせる要因になります。コメントは端的に、修正して欲しいないように沿って記述することが重要です。また、修正指示を行う際には、なぜその修正が必要なのかを説明することも大切です。これにより、開発担当者は修正の意図を理解しやすくなります。
フィードバックを受け入れる姿勢を持つ
開発担当者は、レビュアーからのフィードバックを受け入れる姿勢を持つことが重要です。フィードバックは、コードの品質向上やバグの早期発見に役立つ貴重な情報です。フィードバックに対して感情的にならず、冷静に受け止めることが大切です。
これはレビュアーのコメントの内容にもよります。レビュアーは感情的にならず、丁寧にコメントを記述するように心がけましょう。また、コメントに対して反応をもらっても、それに対して感情的な反応をしないことも大事です。
合意形成のためのディスカッションを行う
PRレビューでコメントが何回か繰り返されるケースがあります。これは、レビュアーと開発担当者の間での合意形成ができていないことが原因です。こういった場合は、オンラインでのディスカッションは一度止め、別途ミーティングの場を設けるなどして、早めの合意形成を目指しましょう。
レビュー対象の明確化
では、実際のPRレビューでは、どのような点に注意してレビューを行うべきでしょうか？以下に、レビュー対象を明確にするためのポイントをいくつか挙げます。
- 何を見ればよいのか
- スコープの明示
- 「すべてに目を通す」必要はあるのか？
何を見ればよいのか？
PRレビューでは、以下のような点に注意してコードを確認します。
- ロジック
- 変数・関数・クラスなどの命名
- 可読性
- テストコード
- ドキュメント
ロジック
コードのロジックは、仕様・要件を満たしているか、明らかなバグがないかを確認します。特に、条件分岐やループ処理などの複雑なロジックは、注意深く確認する必要があります。ロジックの誤りは、バグの原因となることが多いためです。
変数・関数・クラスなどの命名
変数・関数・クラスなどの名前が、プロジェクトの命名規則に従っているか、意味が明確であるかを確認します。命名が適切でないと、コードの可読性が低下し、メンテナンス性が悪化します。Typoのチェックもこの中に含まれます。
プロジェクトが大規模になると、変数名が重複していく可能性があります。そうした中でもわかりやすく命名されているか、意図が明確であるかを確認します。
可読性
組織による開発の場合、コードは一人が編集し続けるわけではありません。他のメンバーでも編集しやすいように、理解しやすいものであるかどうか、可読性が高いかどうかを確認します。
可読性の低いコードは、将来的なメンテナンスや機能追加の際に、理解しづらくなり、バグの温床や技術的負債の原因となります。コメントの有無なども含めて、コードが他のメンバーにとって理解しやすいかどうかを確認しましょう。
テストコード
PRレビューを通したら不具合はないと思われがちですが、そんなことはありません。PRレビューはあくまでも人の目によるチェックであり、そのコードに不具合があるかどうかは判断できません。不具合の有無を確認するためには、テストコードが必要です。
テストコードが十分にあるか、カバレッジが適切かを確認します。特に、重要なロジックや条件分岐が含まれる部分については、テストコードが充実していることが望ましいです。
ドキュメント
コードの変更に伴うドキュメントの更新が行われているかを確認します。特に、APIの変更や新しい機能の追加がある場合は、ドキュメントも更新されていなければなりません。ドキュメントが更新されていないと、将来的なメンテナンスや他のメンバーの理解を妨げる要因となります。
スコープの明示
開発担当者は、PRを作成する際に、レビューのスコープを明示しましょう。具体的には、「これはWIP（Work In Progress）であり、まだ完成していない」「レビュー観点はここだけに絞ってほしい」など、レビュアーに対して明確な指示を提供します。そうしなければ、レビュアーはどの部分に注目すればよいのかが不明瞭になり、負担が増します。
スコープの明治は、そうした負担を軽減し、レビューの効率を高めるために重要です。見て欲しいポイントについては、PRの説明文やコメントで明示することが推奨されます。
「すべてに目を通す」必要はあるのか？
責任感の強いレビュアーほど、PRのコード全体に目を通そうとする傾向があります。しかし、実際にはすべてのコードに目を通す必要はありません。特に大規模なPRや変更が多い場合、すべてのコードを確認することは現実的ではありません。
そのため、以下のようなポイントを考慮して、レビューの範囲を絞ることが重要です。
- 重要なロジックや条件分岐
- 変更の影響範囲
- スコープに明示された部分
レビュアーは、PRの説明文やスコープに基づいて、特に重要な部分に注目してレビューを行うことが推奨されます。すべてのコードに目を通すのは時間的なコストが高く、効率的ではありません。また、機能要件などについて把握していないと、ロジックの成否を判断するのは難しく、時間をかけた割にレビューの品質はあまり向上しません。
大事なポイントとしては、開発担当者が自分の行なった変更に対して、どのような変更であるかを明示的に説明できることと、その説明に沿った変更が行われているかを確認するということです。
まとめ
今回はPRレビューの基本として、そのワークフローと役割分担について解説しました。PRレビューはチーム開発に欠かせないプロセスですが、意外と流れや責任範囲が明確になっていない組織が多いようです。効果的なPRレビューを実現するためにも、今回の記事を参考にして、チーム内での合意形成を進めてください。
続きはコードレビューの基本 その3：レビューガイドラインのすすめ。
おまけ：CodeRabbitの活用
CodeRabbitはAIコードレビューサービスで、GitHubやGitLabと連携してPRに対して自動的にレビューを実行します。開発者はCodeRabbitからの指摘を確認し、修正を行うことで、コードの品質を向上できます。最近リリースされたCodeRabbit for VSCodeを利用することで、ローカルでのレビューも「無料で」可能になりました。
ぜひ、CodeRabbitを活用して、コードレビューの効率化と品質向上を図ってください。
Discussion