PRをレビューする時に気をつけること

普段他の方のPRをレビューする機会があるのですが、その際にどういった観点からコードを評価すれば良いのかについて考えたので以下に記載します。

PRの規模は大きすぎないか

まず確認するのはPRの規模です。具体的にはPRのFile Chagedのところです。
修正ファイル数が少ない方がレビューの負担が軽くなるかと思います。
大体20ファイルを超えてくると見る方もかなり大変になってきますのでPR自体を分割するなどするようにしています。

PRの提出の仕方が互換性の観点から適切か

大きな機能開発などを細かくPR提出する際に個々のPRのみでは互換性が保てない時はPRがdevelop, mainブランチではなく、トピックブランチに向いているかなどを確認するようにしています。

PRの目的（仕様）とコードが乖離してないか

こちらは最も大事な点だと思うのですが、自分でも失念しがちな点です。。
レビュアーとしてはPRのコメントや仕様書のリンクを貼っておいていただけると助かると思います。
一方、レビューイとしては自分が設計+開発をどちらも担当している場合はドキュメントや仕様書に設計方針や仕様を明示し、カウンターパート（非システム部門）と認識を共有しておく必要があると思います。レビューイが開発のみを担当する場合は設計担当者と認識齟齬がないようにしておく必要があると思います。テストコードのみからなるPRをトピックブランチに提出するのも一つの手としてありうるかと思います。また、API開発などではOpenAPIなどを使用することでコードの中で仕様書を管理したりそれをテストコードとして使用したりできるので活用するのも大事です。

テストが網羅的に書けているか

実行コードの仕様としてテストコードは大事です。
なので、よく実行コードとテストコードの修正を別PRで提出する方がいるのですが、できれば一緒に提出していただきたいです。テストが網羅的に書けているかもチェックします。Rubyの場合はSimpleCovがレポートを出してくれるので大変便利です。

パフォーマンスで改善の余地はないか

パフォーマンスも改善の余地がないか確認します。DBに投げられるSQLを確認したり、余分なクエリが発生していないかなどを確認します。Ruby開発の場合はbulletを導入していると便利です。

リリース時に手動作業等の特殊作業が必要でないか

DBのテーブルの型を変更する時やデータパッチを行う場合は自動デプロイの後に手動対応が必要なのでレビューイが意識できていない場合は特にPRのコメントに「FYI」としてメモしたりします。

まとめ

以上がPRレビューの際に気をつけていることでした。他にもあると思うので今後思いついたらまた記事にしていこうと思います。