はじめに
私のチームで行っているコードレビューを行う際に、どのように進めていくかの指針を定めているのですが、普遍的に活用できる内容であるため、この取り組みを紹介します。
コードレビューの指針
目的
- 良いコード、クソコードについての共通認識を持った上でコードレビューを行うことで、チーム全体のプログラミングレベルを引き上げる
心構えについて
- 双方
- 良いコードについて共通認識を持つこと
- レビュイー
- コードの変更目的(Why)、具体的な変更内容(What)を明確にする
- 指摘は人格否定ではないことを理解し、素直な気持ちで受け入れる
- レビュアー
- 不明な点は素直に質問する
- 感じた点は、我が身に返ることを恐れず指摘する
- なぜ悪いのか論理的に説明する
- 良かったら褒める
レビュー観点
- 設計
- システムに対して適切な設計になっているか
- 機能
- 仕様に沿った or 開発者が意図した実装になっているか
- 複雑さ
-できるだけシンプルになっているか
- 第三者が再利用、修正しやすい形式になっているか
- 現時点で必要以上の機能が実装されていないか([YAGNI])
- 冗長な条件文で記述されていないか - テスト
- 実装内容に対して、適切なテストを実施しているか / テストコードが実装されているか
- 命名
- 変数やクラス、メソッドなどに明確な名前が付けられているか
- 関数の役割や変数の内容が理解でき、適度な長さになっているか
- 変数やクラス、メソッドなどに明確な名前が付けられているか
- コメント
- コメントは明確で分かりやすいか
- コード自体から読み取れない、何故その処理が必要なのか背景を説明するのに活用する
- 処理内容についてのコメントは極力避け、コードをシンプルにすることで対応する
- コメントは明確で分かりやすいか
- スタイル
- コードはスタイルガイドに従っているか
- ドキュメント
- 関連するドキュメントは更新されているか
具体的な進め方
- レビュイー
- 細かくフィードバックを得るため、レビュー依頼を小さい単位でできるように心掛ける
- レビュー前に、上記の観点に沿った実装ができているかを確認する
- コードの変更内容をまとめて、レビュワーに伝える
- 後述のテンプレート
- レビュワー
- 変更されたコードについて、行っている処理を全て理解する
- 不明な点については、どういう処理なのか、どういう意図なのかを質問する
- 指摘点について、論理的に理由を説明し、具体的にどのように修正すべきかのフィードバックする
- 指摘点だけでなく良いところもフィードバックする
決めておくべき事柄
レビューに関する前提条件
開発フローの中で、どのタイミングでレビューをするか、また事前に満たしておくべき基準といった標準的な決め事を定めておきます。
例えば、GitFlowスタイルの開発フローにおけるreleaseブランチへのマージ時に、事前に自動テストの通過を確認しておくなど。
コードレビューテンプレート
GitのPull Requestのテンプレートとして記載しています。
可能であれば、タスク管理ツール等と連携するのが良いと思います。
- 概要
- 実装した機能についてまとめる
- 対応する課題チケットのリンクを添付する
- 主要変更箇所
- 修正に伴う影響範囲をチェックリストで記載する
- 留意事項
- 必要であれば、レビュワーに特に確認をお願いしたい部分や伝えておくべき情報を書く
Linter
コードレビューに直接的に関係ありませんが、チーム内で共通のLinterを設定してフォーマットを統一することで、手軽にコーディングスタイルが統一でき、それに伴い可読性が上がります。