修正箇所の指摘

レビュー観点#

通常のレビューは上の観点から順に実施され、途中で問題が露見した場合、その観点でレビューがストップします。 上位観点が誤っている場合、後続を確認する上での前提条件が崩れるため、まず先に露見した観点での改善対応が必要となります。

  1. 実装者の仕様理解が合っているか(仕様理解)
  2. 期待値と実装者の変更内容が一致しているか(作業理解)
  3. 期待通りに動作するか(動作確認)
  4. 関連箇所がデグレしていないか(動作確認)
  5. ソースコードの品質に問題ないか(設計)
  6. 保守不能にならないか(保守)
  7. すでにある機能を再開発していないか(保守)

従って、レビューの指摘回数を減らし、レビューを早く通すためには、1〜5の指摘が発生しないように実装者が作業することが望ましいと言えます。

仕様理解#

仕様理解が誤っている場合、レビュアーは実装者へ正しい仕様理解を促す必要があります。

作業理解#

作業理解が誤っている場合、レビュアーから実装者へ、期待値と変更内容の紐付けについて理解を促す必要があります。 また、期待値と異なる変更内容を追加する場合、レビュー依頼を行う前に実装者が主体となり、PMと認識を一致させておく必要があります。

動作確認#

以下に問題がある場合、レビュアーから実装者へ、動作不具合を伝えます。

  • 期待値(仕様)どおりに動作するか
  • ソースコード変更の影響箇所で、デグレが発生していないか

設計#

以下に問題がある場合、レビュアーから実装者へ、設計上の問題を伝えます。

  • 例外が発生した場合であっても操作不能に陥らないか
  • メモリを大量消費しないか
  • 効率の悪い処理をしていないか

保守#

以下に問題がある場合、レビュアーから実装者へ、保守上の問題を伝えます。

  • コーディングルールに即しているか
  • 特殊なロジックであればコメントが記載されているか(第三者がソースコードの修正方針を検討できるか)

指摘の伝え方#

レビュー指摘を文字で最低限の話を伝えるだけでは、実装者に心理的負担かかる可能性があります。 レビュアーは指摘内容の伝え方を工夫し、実装者がスムーズに作業に移れるようコミュニケーションをとることが必要です。

  • 作業への感謝の気持ちを伝える
  • プルリクエストの内容に対して指摘する(パーソナリティへの指摘は厳禁)
  • エントリーレベルの実装者へはティーチングスタイルで指摘する(誤りの内容と、修正方針を伝える)
  • 中堅以上の実装者へはコーチングスタイルで指摘するのが望ましい(誤りの内容を伝え、修正方針と再発防止策を考えるよう促す)