自己流コードレビューのやり方
ソフトウェア開発にまつわるコードレビューってありますよね。
ここ数年、僕が自己流で試行錯誤しながらやっているコードレビューの細かい作業詳細を記録しておきます。
レビュー依頼
基本的にある程度の機能単位でのチケット駆動開発でプロジェクトを進めているのですが、実装、ユニットテスト、設計書作成が終わった段階で機能のチケットを担当者がクローズしてレビュー依頼チケットを別途登録します。
その際に、閉じたチケットで作ったソースがレビュー対象になるので、コミットしたリビジョンなり、ソースファイル名なんかをレビューチケットに書いておきます。
レビュー着手
レビュー依頼を受けた、レビュー担当は依頼のチケットを見ながらソースを見る前に以下の確認をします。
ビルドやテストでこけるような場合は、正直言ってレビューに耐えるレベルではないのでその時点で開発担当者にレビューチケットの担当者を変更し突き返します。
で、カバレッジや静的解析ツールの結果はその内容と合わせてレビューとして指摘事項にするなり、事情を開発担当者に聞くなりしながらレビュー結果に盛り込みます。
ここまでが前処理
レビューとしてのコードリーディング
基本的にはソースはトップダウンというか呼び出し階層の上位からの読み下しで見ながらレビューを行います。
で、その読み下し方なんですが単純にソースを呼び出しもとから順に読んで行ってもたいていは脳のバッファがオーバーフローして途中から入る分だけ抜けていく状態になってしまいます。
なので、記憶に頼らずメモを残すのですがそのメモの残し方
- UMLのクラス図を書きながらクラス構成を理解する*5
- クラス図と並行してシーケンス図も呼び出し階層順に書いていきます。*6
- 呼び出し階層の網羅が終わったら気になるところをメモリながら*7テストケースも読む。
- クラス図、シーケンス図を総合した上で脳内のCode CompleteやClean Codeといった過去の書籍の教えと照合する
- 設計書を見ながら、仕様との整合性をチェックする
こういった、形でソースをレビューします。
設計書は単体でレビューすることもありますが、最近はソースと突き合わせる形で一式のレビューをしてしまいます。
開発担当者へのフィードバック
これまでのレビュー結果をレビューチケットに記入する形でフィードバックします。
この際、以下のことに気を配ります。
- たとえまずい実装があったとしても人格攻撃はしない。*8
- 指摘する箇所には個人の意見として変更案を追記する。
- あくまで、開発担当者が納得して受け入れたものがソースに反映される。
といったところでしょうか。
他のブログでも見かけましたが、レビュアーとレビュイーに上下関係がなく、別の機能では逆転とかしてもちゃんと機能するようにチームビルディングを心がけています。
という、覚書。
*1:Jenkinsでデイリービルドはしていますが、一応手元でも最新ソース一式をビルドします。
*2:ちなみに、今回はC#の案件だったのでNUnitでした。
*3:自分たちのプロジェクトではOpen Coverでカバレッジを取っています。
*5:astah communityの力を利用してなるべく労力をかけずにクラス図を厳密でなくてよいのでなんとなく作ります。
*6:条件分岐やループなんかはシーケンス図にメモを追加で記述する感じ
*7:基本的にMarkDownで構造化しながらメモを残します。
*8:『ここがクソだ』とか言いたくなったら『こういうデメリットがあるよ』みたいなまろやかな表現に変えたり。
