2015/04版
現コードレビュー運用方法とその課題を記述する
- 情報システム部門として社内業務システムを内製している
- 言語はJava
- 毎日09:30から、前日プッシュ分のソースコードをレビューする(PRレビューではない)
- レビュアは部門長
- 開発者は全員参加(同席、もしくはSkype)
- 長くて30分程度
- レビュア端末上の画面を画面共有しながらレビューする
- 出社者もリモートワーク者も自端末でSkype経由でレビュー画面を参照する
- マイク買ったほうが雑音入らなくて
- 前日差分を参照するのが容易
- upsourceでレビュー管理ができるが利用していない
- 前日差分以外のソースを参照するときには、IDEの方が便利なのでEclipseを利用している
- 前日ソース差分から、
- ぴよぴよしているところがあったら指摘して修正してもらう
- どういった点がにぴよぴよしているのか、どう修正すべきかを皆で共有する
- 第三者の視点からソースを見て、理解しにくいところを指摘している
- クラス図、状態遷移図が適切かどうか確認する
- クラス図から機能性について、またその機能性がユーザの要件を満たしているかどうかを確認
- 状態遷移図から遷移想定漏れがないかどうかを確認
- クラス責務内容が、定めたクラス配置パッケージに配置されているかどうかを確認する
- 値オブジェクト, ポリシー, プロセス, 状態遷移管理など、切り出すべき責務がエンティティやコントローラに直接記述されていないか
- またそれらのクラスが適切なパッケージに配置されているか
- 修正差分ソースに対する指摘のため、主にマイクロマイクロリファクタリングの指摘になりがち
- メソッド抽出、説明変数の設置、クラス名・メソッド名の変更など
- ドメインのクラス構成、サービスのクラス構成などはソース差分からは把握しにくい(後述2.参照)
- クラス図に対してソースコードが適切に実装されているかどうかは前日差分からはわからないため、定期的に個別レビューが必要になる
- ソース差分から責務の確認は難しい場合が多い
- また一度見逃すとそのまま埋もれてしまうため、定期的に個別レビューが必要になる
- コードレビューは口頭で行うだけでチケット作成していない
- あるいはソースコード上に、直接 //TODO として記載するだけでチケット作成していない
- コードレビュー内容に関してディスカッションしたい場合に、それを行う場(チケット)が存在しない
- できればレビュアーがレビュー時にチケットを作成するのが望ましいが。。
- レビュー時にプロジェクトの進捗状況や課題を確認しているが、当該プロジェクト以外の関係者にとっては関係のない話で時間を奪う結果となっている
- レビュー時に初めてコードを見て指摘をするのは難しい、特にクラス責務や構造に関するものはある程度事前に他のクラスを含めてコードを網羅して見ておく必要がありコストが大きい
- レビューすべきコミットかどうかがコミットログから判断できない
- 定期的に個別レビューを行っているが、このレビューは当該開発者に直接伝えられ、他者との共有がない
- テストコードのレビューが出来ていない
- レビュアーを増やす
- 現在口頭のみの指摘でその記録は行っていない
- 手間でもコメント内容を記録し、後から分析できるようにしておくことで、指摘内容の分析やコーディングルールに結びつけることができる
- Upsourceのコメント機能を利用してみる
- 口伝となっているコーディング規約を文書化する
- 頻出するレビュー指摘に関しては、随時追記する
- レビューすべきかどうかの判断情報をコミットログに含める
- レビューを効率的に行えるようにコミットログに価値の高い情報を含める
- レビューし易い粒度でコミットする
- コミット単位もレビュー対象とする
- 1行目に簡潔な概要を、3行目以後に詳細を書く
- コミットログにmarkdown記述が可能なので、わかりやすく書こう!
参考
- 当該プロジェクトの進捗や課題管理など、コードに関連しない内容に関してはレビュー時には(できるだけ)行わない
- 一ヶ月単位などで相互レビュー者を定め、チケット単位でお互いにレビューし合う
- 相互レビューすることで、自身のコード記述に対しても大きな気づきを得られる
- コミット単位やチケットの切り方、読みやすいコードなど
- デイリーレビューで拾いきれないクラス構成のレビューなど、別枠のレビュー会として設け、そこで行う
- CheckStyle, FindBugs, テストカバレッジの取得
- レビューする以前の指摘項目に関しては、開発者自身がメトリクスから把握できる環境を整備する