プログラミング初心者がアーキテクトっぽく語る

見苦しい記事も多数あるとは思いますが訂正しつつブログと共に成長していければと思います

コードレビュー

今まで軽視されがちだったがコードレビューだが、最近ではその有効性が広く認知されるようになり重要性が増している。

「GibHubからリクエストが飛んでくるあれでしょ」と思ったあなた。コードレビューというのはもっと深く広いものだと知ってください。突き詰めれば日々のコミュニケーションですらコードレビューとなり製品の品質を向上させるのです。


コードレビューの目的

  • バグの検出
  • 品質の向上
  • 知識の共有
  • メンバーの教育

確認すること

  • どのように実装されているか
    • 規約を守っているか
    • 可読性はあるか
    • 読んでみて?な処理、構造はないか?
  • テストはあるか
  • 後述するレビューの単位によって確認の粒度は異なる

所要時間

  • 後述する単位や形式によって異なる
  • Pull Requestベースのレビューであれば1件60〜90分で済む程度の大きさの変更であることが好ましい

コードレビューの単位

コミット単位

  • コミット単位でレビューする
  • 細かい粒度で確認できる
  • 一つの仕様変更が複数のコミットで成立している場合、全体を俯瞰できない

複数のコミットに対するレビュー

  • Pull Requestなどの確認がこれに相当する
  • 一連のコミットで意図されるソフトウェアの仕様変更全体を俯瞰できる
  • 細かい粒度で見るにはバッチ単位の方がよい

ブランチ単位

  • 細かいことは見えない
  • ブランチ全体の方向性、設計方針など設計に関わるレビューが中心になる
    • むしろコードを書く前の段階で議論すべき?

形式

インスペクション

  • 会議形式、厳格、大々的、高コスト

チームレビュー

  • インスペクションほどではないが厳格で高コスト

ウォークスルー

  • 作成者が成果物の説明をする

ペアレビュー

  • 二人でレビューし合う
  • 密室での議論なので抜け、漏れのリスクがある

パスアラウンド

  • GitHubなどを使って非同期的に同時進行型で行う

アドホック

  • その場ですぐに相談する

レビュータイミング

プレコミット

  • パッチにまとめてレビューに回す
  • 問題を抱えたコードがコミットされるリスクを回避できる
  • パッチ(差分)なのでプレマージに比べると確認しにくい
  • レビューが停滞しコードがコミットされなくなる心配がある

プレマージ

  • コミットされた後なので確認しやすい
  • プレコミット同様にブロッキングの心配がある

ポストコミット

  • 昔は一般的だったらしい
  • 形骸化しやすい

ツール

以前はRietveltやGerrit(いずれもGoogle起源)、Phabricator(Facebook)など色々あった。

現在はGitHubやGitLabなどWebベースのバージョン管理システムを利用したレビューが一般的になっている。レビューを依頼する機能はGitHubだとPull Request、GitLabだとMerge Requetという。