長いPRは避ける

長いPRは避ける

  • 新規開発とかをしていると起こりがちなのが数千行にも及ぶ差分
  • 数千行ものコードを正しくレビューできる人間なんてそうそういない
  • 少なくとも自分は何かを見落とす自信がある

どうすればよかったのか?

  • 機能ごとの開発に切り分ける
  • それでも差分が大きくなりそうであれば、以下の様な単位に切り分ける
  • 画面遷移などのロジックを伴わない処理
  • 使うであろうDBやキャッシュ、ストレージへのアクセス
  • ビジネスロジックの簡単な作り
  • 見てもらい単位で関数ごと、もしく関数群
  • バリデーション
  • 異常系の漏れがないかのチェック

何が嬉しいか

  • 何か根本的に間違ったコードや設計があった場合に早期発見が可能になる
  • レビューする側が仕様を正しく認識していない場合は仕様の細かい点まで理解しやすい
  • どういうI/Oになっているのか?
  • どんなSQLが発行されて、それらはパフォーマンス劣化を起こさないか?
  • バリデーションに漏れはないか?適切なバリデーションが行われているか?
  • レビューのためのコンテキストスイッチの切り替えは大変だが、20−30min程度であれば意外と息抜きになる

注意したいこと

  • 細かすぎても逆にストレスを与えるかも知れない
  • 実際にやってみて、合意がとれてから行う方がよいだろうとは思う
  • 合意がとれなかったことは今までになかったが
  • こうしましょうねと見せても相変わらず巨大な差分を生み出す人間はいたが。。

どうやるか

  • 目安は200−300行程度を上限にする
  • 見てもらいたい単位を設計時に明らかにしておく
  • レビュー観点を満たすだけのコード単位でコミットすることは簡単
  • また、その単位を習得していくことによって設計等の考え方も日々、変わってくる
  • 一つのことに集中してレビューしてもらえる様に保つ