コードレビューについて考える

はじめに

アプリケーション開発業務におけるコードレビューはコードの正しさや質そして一貫性を保ち、それらと同時にコードに対するチームとしての共有知を作り上げる良いプラクティスです。
企業によっては、Pull Requestを1〜2人以上承認がないとmergeができないなどの設定をしているところもあると思います。

ただ、この2人以上という設定にすること、承認がないとmergeできない設定にすること。
これによってmerge待ちの状態が発生してしまうのではないでしょうか。

ただ、Pull Requestのテンプレートを作成して、変更点を記載しておく、といった方法で
レビューする側が不明点はなくなると思うので、
それでもレビュー待ちが発生しているのであれば、
チームに詳しい人がいない、各人でレビューする時間もないほど忙しいといった状態が生まれているなどが考えられるのではないでしょうか。

目的

• コードの正しさを保つ

  • 誤っていそうなコードやコメントはないか
  • その実装方法によって想定外の仕様上の制約を作っていないか
  • 適切なインターフェースをデザインできているか

• コードの質を保つ

  • パフォーマンス上の問題やセキュリティ上の問題を起こし得る実装になっていないか

• それは様々な開発上の背景のもと、現状で許容できるかできないか

  • コードの追加や変更に対して適切に自動テストの実装や修正が行われているか

• コードの一貫性を保つ

  • 命名規則が適切に適用されているか

承認機能

Pull Requestをもとにコードをレビューするとき、
チーム内でそれを行っている場合においてその承認機能がどのようにチームに作用するのかを考えます。

Pull Requestはレビューに限らず、コードの変更をチーム内外に知らせるために作成されます。
その際に、前述したような目的に沿ってレビューが行われることが期待されていて、
コードの変更を行った人は多くの場合はそれを期待して、Pull Requestを作成していると思います。

承認の意味

Approveの制限人数をかけているのはどういった意味があるのか？
レビュアーがその変更の取り込みを承認したということですが、
Pull Requestを作成して、Reviewerに追加すれば、共有知は作り上げられるのではないでしょうか。
承認の制限を求める意味はあるのでしょうか。

まとめ

レビュー待ちは、少しづつでも減らせていけるなら減らしていった方がいいと思っていて、
まだ答えとしては出ていないのですが、承認の制限までは不要なのでは？と考えていました。

例えば、READMEなど都度更新される内容に関しては、Reviewは不要にするなど
対応内容によって変わると思います。

一律、2名以上の承認が必須などに適用してしまうと、
スピード感が失われてしまうので、早めに取り込んでいって、
実装方針を先に決めておき、不具合が発生してしまった場合は、
修正したPull Requestをすぐさま作成していく方が
開発のスピード感は失われないのでは？
と思いました。
ただ、レビューでの指摘が多い場合は取り込まない方がいいとは思っています。