Skip to content

Instantly share code, notes, and snippets.

@asufana
Last active June 4, 2018 09:00
Show Gist options
  • Save asufana/96fbbcf1308b6bddb070 to your computer and use it in GitHub Desktop.
Save asufana/96fbbcf1308b6bddb070 to your computer and use it in GitHub Desktop.
コードレビュー運用について

コードレビュー運用について

2015/04版

現コードレビュー運用方法とその課題を記述する

  • 情報システム部門として社内業務システムを内製している
  • 言語はJava

現レビュー運用

  • 毎日09:30から、前日プッシュ分のソースコードをレビューする(PRレビューではない)
  • レビュアは部門長
  • 開発者は全員参加(同席、もしくはSkype)
  • 長くて30分程度

利用しているツール

Skype:画面共有

  • レビュア端末上の画面を画面共有しながらレビューする
  • 出社者もリモートワーク者も自端末でSkype経由でレビュー画面を参照する
  • マイク買ったほうが雑音入らなくて

Upsource:前日プッシュコードの参照

  • 前日差分を参照するのが容易
  • upsourceでレビュー管理ができるが利用していない

Eclipse:クラス構成の確認

  • 前日差分以外のソースを参照するときには、IDEの方が便利なのでEclipseを利用している

コードレビューの目的

1. ひよコードの指摘

  • 前日ソース差分から、
  • ぴよぴよしているところがあったら指摘して修正してもらう
  • どういった点がにぴよぴよしているのか、どう修正すべきかを皆で共有する
  • 第三者の視点からソースを見て、理解しにくいところを指摘している

2. 静的構造の確認

  • クラス図、状態遷移図が適切かどうか確認する
  • クラス図から機能性について、またその機能性がユーザの要件を満たしているかどうかを確認
  • 状態遷移図から遷移想定漏れがないかどうかを確認

3. クラス責務と配置の確認

  • クラス責務内容が、定めたクラス配置パッケージに配置されているかどうかを確認する
  • 値オブジェクト, ポリシー, プロセス, 状態遷移管理など、切り出すべき責務がエンティティやコントローラに直接記述されていないか
  • またそれらのクラスが適切なパッケージに配置されているか

現運用での問題

1. ひよコードの指摘

  • 修正差分ソースに対する指摘のため、主にマイクロマイクロリファクタリングの指摘になりがち
  • メソッド抽出、説明変数の設置、クラス名・メソッド名の変更など
  • ドメインのクラス構成、サービスのクラス構成などはソース差分からは把握しにくい(後述2.参照)

2. 静的構造の確認

  • クラス図に対してソースコードが適切に実装されているかどうかは前日差分からはわからないため、定期的に個別レビューが必要になる

3. クラス責務と配置の確認

  • ソース差分から責務の確認は難しい場合が多い
  • また一度見逃すとそのまま埋もれてしまうため、定期的に個別レビューが必要になる

4. チケット作成

  • コードレビューは口頭で行うだけでチケット作成していない
  • あるいはソースコード上に、直接 //TODO として記載するだけでチケット作成していない
  • コードレビュー内容に関してディスカッションしたい場合に、それを行う場(チケット)が存在しない
  • できればレビュアーがレビュー時にチケットを作成するのが望ましいが。。

5. その他

  • レビュー時にプロジェクトの進捗状況や課題を確認しているが、当該プロジェクト以外の関係者にとっては関係のない話で時間を奪う結果となっている
  • レビュー時に初めてコードを見て指摘をするのは難しい、特にクラス責務や構造に関するものはある程度事前に他のクラスを含めてコードを網羅して見ておく必要がありコストが大きい
  • レビューすべきコミットかどうかがコミットログから判断できない
  • 定期的に個別レビューを行っているが、このレビューは当該開発者に直接伝えられ、他者との共有がない
  • テストコードのレビューが出来ていない
  • レビュアーを増やす

レビューに関する運用改善案

レビューコメントの記録

  • 現在口頭のみの指摘でその記録は行っていない
  • 手間でもコメント内容を記録し、後から分析できるようにしておくことで、指摘内容の分析やコーディングルールに結びつけることができる
  • Upsourceのコメント機能を利用してみる

コーディングルールの文書化

  • 口伝となっているコーディング規約を文書化する
  • 頻出するレビュー指摘に関しては、随時追記する

コミットログのルール

  • レビューすべきかどうかの判断情報をコミットログに含める
  • レビューを効率的に行えるようにコミットログに価値の高い情報を含める
  • レビューし易い粒度でコミットする
  • コミット単位もレビュー対象とする
  • 1行目に簡潔な概要を、3行目以後に詳細を書く
  • コミットログにmarkdown記述が可能なので、わかりやすく書こう!

参考

コードに関係のない話題をデイリーレビュー中に行わない

  • 当該プロジェクトの進捗や課題管理など、コードに関連しない内容に関してはレビュー時には(できるだけ)行わない

相互レビュー

  • 一ヶ月単位などで相互レビュー者を定め、チケット単位でお互いにレビューし合う
  • 相互レビューすることで、自身のコード記述に対しても大きな気づきを得られる
  • コミット単位やチケットの切り方、読みやすいコードなど

デイリーレビュー以外の定期レビュー会を別途設ける

  • デイリーレビューで拾いきれないクラス構成のレビューなど、別枠のレビュー会として設け、そこで行う

静的メトリクスの取得

  • CheckStyle, FindBugs, テストカバレッジの取得
  • レビューする以前の指摘項目に関しては、開発者自身がメトリクスから把握できる環境を整備する
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment