チーム開発におけるコードレビューについて、事前にチェックリストをつくり共有しておくことで、円滑にレビューできるだけでなく、開発者もよいコードを書けるようになります。 ここではチェックリストの例と、それを用いたコードレビューのやり方について解説します。

メリット

事前にチェックリストを作成することで、以下のようなメリットがあります。

  • 不要なやりとりが減ることで、コミュニケーションコストが下がる
  • リストを満たすことを開発者自身が確認することで、コードの品質が上がる
  • 指摘すべき点が定義されているので、いいづらいことも言いやすい

準備

以下が曖昧なままレビューを行なうと、効率的にレビューが行なえません。 時間を無駄にしないよう、はっきりとさせておきましょう。

  • プルリクエストの本文に不明な点はないか
  • レビューのコンテキストを明確にする
    • WIP段階：ビジネスロジックの視点（全体像）で指摘する
    • マージ前：コードの質（細部）を含めて指摘する
  • diffの量は適切か

チェックリスト

以下にチェックリストの例を示します。 コードレビューの際は、以下の各項目において問題がないかをチェックします。

  • ブランチのテーマとは関係のないコードが含まれていないか
  • コミットメッセージは簡潔かつ分かりやすいか
  • 責務に応じてリファクタリングされているか
  • メソッドを不必要にpublicにしていないか
  • コードは読みやすいか/分かりやすいか
  • クラス/メソッド/変数の名前は適切か
  • 既存コードとの重複はないか
  • 既存コードに影響を及ぼさないか
  • コメントが必要な箇所はないか
  • デバッグ用のコードは残っていないか
  • もっとよいコードにできないか
  • ファイル名は適切か
  • 不要なファイルをコミットしていないか
  • typoはないか
  • コーディング規約に則っているか
  • バリデーションは適切か
  • セキュリティ上のリスクはないか
  • フレームワークのレールから外れていないか
  • ライブラリで代替できる処理はないか
  • 将来的な負債は予期されないか
  • トランザクションが必要な箇所はないか
  • キャッシュが必要な箇所はないか
  • 不必要なSQLを発行していないか
  • テストケースは十分か
  • 境界条件で正常に動作するか
  • エラーハンドリングは必要な箇所で行なわれているか

チェック後の対応

チェックリストを確認したら、開発者/レビュアーは以下をふまえた上で相手に対応します。

開発者

  • 自分が実装した差分がチェックリストをすべて満たすことを確認してからプルリクエストを出す
  • 懸念点があれば、その点をプルリクエスト本文に書く

レビュアー

  • チェックリストの各項目について、コードをよりよくするために必要なことをコメントする
  • 自分の感覚ではなく、論理的に指摘する
  • 相手のレベルに合わせて分かりやすくコメントする
  • 重要度を明示する
    • MUST: 必ず対応すべき
    • IMO: 私の意見ですが……
    • nits: 細かい指摘
  • 相手を肯定する
  • コードに対して指摘し、決して人に対して指摘しない

注意事項

上記のチェックリストは、あくまで例です。 チームやプロジェクトの性質によって、内容は変わってくると思います。 必要に応じて加筆修正してください。

また、開発者全員で共有し、全員が理解・納得しなければ意味がありません。 疑問などがあれば議論し、必要に応じて更新していきましょう。

おわりに

チェックリストをチームできちんと運用できれば、よりよいコードをすばやくプロダクトに反映できるようになります。 チームのよい習慣となるよう、参考になればと思います。