レビューがしやすいPRの粒度について

こんにちは。
ピクトリンク事業部でwebアプリケーション開発を担当している西村です🧑‍💻
この記事はフリューテックブログリレー2日目の記事になります。

テックブログリレー1日目の記事はRieさんの「アジャイル開発の見積もりの考え方を非エンジニアに説明してみた件」です！
アジャイルの見積もりについてとても分かりやすく簡潔にまとめてくださっているので、是非読んでみてください！🙌

今回私の記事では「レビューがしやすいPRの粒度について」というお題でおはなししてみようかと思います！

まず初めに、PR*1の粒度は1つの関心ごとに絞ることが大切だと考えています。
そして、そのPRにはテストの実装もセットにすることが重要です。

「一つの関心ごと」とは、開発作業を分解した際の意味のあるまとまりのことを指します。
つまり、「この変更は何のためのものか？」が一言で説明できる単位になります。
今回はピクトリンクで提供している以下の画面で分解のやり方を考えてみましょう。

画面の仕様はWebAPIから筐体一覧情報を取得し、一覧表示します。
「証明プリ」、「ピクトリンク連携」ボタンをタップすることで対応している筐体のみを表示するフィルター機能が付いています。

実装を行うにあたって例えば下記のような関心ごとに分解することができます。

上記の分解の仕方だと、1画面作成で4つのPRを作成する形になります。
分解の仕方は個人やチームのレベル感、考え方によって異なるとは思いますが、
上記のような関心ごとで分解することによってコードレビューの負荷を分散させ、レビュアーに效率良くレビューしてもらうことができます。

PRを出すときに、「一言で何をしたかを説明できるか？」がポイントになるので、
もし説明が長くなるならそれはPRを分割する合図なのかもしれません。

1つのPRを1つの関心ごとに限定することで、以下のようなメリットがあります。

変更の影響範囲が限定される
変更の範囲が広いと複数の観点が絡み合い、確認しないといけない項目が増えていきます。
変更内容が明確になるので、変更に対する動作の誤解やバグの見逃しを減らすことに繋がります。
レビュアーの認知負荷が軽減される
一つの関心ごとに絞られることで、レビュアーは単一の変更点に集中することができます。

変更の影響範囲が限定される
例えば「UI実装」と「API通信を行い一覧表示をする繋ぎこみ」を同時に実装した際、
一覧表示の実装に詰まってしまった際にどの部分の実装が上手くいっていないのかが分かりずらい状況です。
範囲を絞ることで影響も少なくなるので、それが実装のしやすさに繋がります。
開発者の認知負荷が軽減される
一つの関心ごとに絞られることで、実装者は単一の変更点に集中することができます。
複数の観点を混在させないことで、不具合を埋め込むことを防止できます。
デバッグが容易になる
バグが発生した際に、PRが小さければ「どの変更が問題を引き起こしたのか」を特定しやすくなります。
小さいPRごとにテストを行うことで、バグの発生を未然に防ぐことも可能になります。
コードのマージがスムーズになる
小さいPRであれば、コードレビューの時間も短くなり、早くマージできます。
大きなPRではコンフリクトが発生しやすくなりますが、小さいPRでは変更範囲が限られているため、コンフリクトのリスクも低くなります。

実装とテストを1つのPRにまとめることで、以下のようなメリットがあります。

テストの実装漏れを無くす
実装とテストのPRが分かれることで、テストケースが網羅されているかが非常に見えづらくなります。
また一度に複数の観点のテスト実装することで、テストケースの漏れにも繋がります。
実装部分のテストもPRに一緒に含める方針にすることで、
テストカバレッジの向上や予期せぬ不具合の見逃しを防止することができます。
コードの信頼性を確保する
PRが実装のみだとレビューだけでは気づけない問題が生じます。
同時にテストを実装していることで、テスト作成した部分の動作を担保することができるので、コードの信頼性が向上します。
自信を持ってコードを変更することができる
自身の変更点以外のテストが拡充していることで、自分の変更による不具合を検知しやすくなります。
変更の影響をすぐに把握できるため、余計な心配をせずに変更部分に集中することができます。