こんにちは！
STORES 株式会社でエンジニアをしています、永尾です。

突然ですが、過去誰しもが不適切に大きな Pull Request によって悲しい気持ちになったご経験があるかと思います。
自分にもその経験があります。

皆様もご存知の通り、大きな Pull Request には多くのデメリットがあります。その一つが、レビューコストです。

レビューコストが大きい Pull Request は、マージまでのリードタイムが長くなります。

余談ですが、私が所属するチームではカンバンのステータスに「Sprint Backlog」「In Progress」「In Review」「Done」を設けています。

ある日、「Sprint Backlog」ステータスにあるチケットに着手し、ステータスを「In Progress」へ移動させました。
その後、実装を終えて Pull Request を作成し、ステータスを「In Review」へ移動させました。
ここまではとても順調だったのですが、私の Pull Request が大きかったために、チケットは「In Review」に長く滞在することになりました。

Pull Request の作成までが如何に早く行えても、結局それをリリースしなければユーザーへの価値提供にはならず、自分も別のタスクに着手することが出来ません。

途中までは順調だったことも相まり、より悲しい気持ちになったことを覚えています。

今後の自分や、この記事を読んでくださった方の脱悲しみのために、小さく適切な Pull Request を作成するための方法をこの記事では書かせていただきたいと思います。

小さな Pull Request の利点

本記事でいう小さな Pull Request とは diff の量ではなく粒度が小さいことを指します。

Pull Request を小さく適切な粒度で作成することには多くのメリットがあります。

まず、レビューが迅速かつ効率的になります。
小さな Pull Request は変更内容が明確で理解しやすいため、レビュワーが迅速にフィードバックを行うことが出来ます。これにより、バグや問題の早期発見が可能となり、修正も迅速に行うことが出来ます。

次に、チーム開発を行う上で他の開発への影響を減らすことが出来ます。
大きな Pull Request はコンフリクトが発生しやすく、解消に時間がかかることがありますが、小さな Pull Request はコンフリクトの発生そのもののリスクが低くなります。

さらに、開発チームの生産性が向上します。
小さな Pull Request を意識した開発を行うことで、機能追加や修正が迅速にリリースされ、ユーザーに価値を提供するサイクルが短くなります。
これにより機能や修正のリリースに対してユーザーからのフィードバックも迅速に受け取れるようになります。

これらの理由から小さな Pull Request を意識することでプロジェクトの品質向上にも繋げることが出来ます。

小さな Pull Request を作成するために

単一の目的にする

Pull Request は、単一の機能の修正、または改善に集中することにより、レビュワーが見たときに変更内容が理解しやすく、レビューコストを抑えることができます。
レビューコストの低い Pull Request はレビュワーにとっても Approve しやすく、マージまでのリードタイムが短くなりやすいです。

反対に複数の目的を含む Pull Request は、変更が複雑化してしまうため避けるべきです。

機能追加のための開発を行なっていると、稀にバグを見つけてしまうことがあるかと思います。
その修正を機能追加と同じ Pull Request に含めてしまうと、その Pull Request が複数の目的を持つことになります。

機能追加の観点では問題ない変更であっても、バグ修正の観点から見て改善の余地がある場合、機能追加以外の目的のためにレビュワーとのやり取りが続き、リードタイムが長くなります。

責務毎に分割する

機能実装を行う際には、責務毎に Pull Request を分けることでレビューコストを抑えることが出来ます。
例えばフロントエンドの開発において、新しいページを追加する場合、マークアップとロジックで分けて実装をします。

これによりレビュワーは変更を理解やすくなると共に、スタイルと動作の検証を分けて行うことができるため早期にフィードバックを得ることができます。
バックエンドにおいても、API とデータベースに関する変更などで分けて実装を行うことができるため同様のことが言えます。

タスクに着手する前に、一度サブタスクとして責務毎に分けることで Pull Request の粒度をイメージすることができるため、サブタスクや、TODO リストの活用もおすすめです。

フューチャートグルを活用する

フューチャートグルとは一言で説明すると「機能のオンオフを管理する仕組み」のことです。
この仕組みを使用することでトランクベース開発に近いメリットを得ることができます。

dora.dev

大きな変更を複数の小さな Pull Request に分けることにより、以下のような利点もあります。

最新のコードと乖離が少ない状態で開発が進められるので、差分起因のデグレやコンフリクトの発生リスクを減らせる
大規模なマージに伴うコードフリーズを避けることが出来る
問題が生じた際に原因の切り分け、切り戻しが容易になる

const FeatureToggle = {
  // リリース前
  goodNewFeatureEnabled: false;
  // リリース済
  greatNewFeatureEnabled: true;
  ...
}


if( FeatureToggle.goodNewFeatureEnabled ){
  //リリース前の機能に関する処理
} else {
  // 既存処理
}

リリース前の処理に関してはエラーを投げるようにしておくことで他の開発者にも未実装であることが明示でき、加えて不具合を検知しやすくなるなどのメリットもあります。

if( IsNewFeatureEnabled? ) throw new Error('Not implemented.')

このように便利なフューチャートグルですが、トグルの管理が煩雑になりがちで、古いトグルを適切に削除しないと技術的負債になる可能性があります。
また、過度なトグルの使用はコードの可読性を低下させることがあるため注意が必要です。

テストとプロダクトコードの実装を分ける

未実装の機能や部分的に完成していないプロダクトコードについては、テストをスキップさせることで先行して実装することができます。
一例として Vitest では todo メソッドがあり、以下はサンプルコードです。

import { describe, it, expect } from 'vitest';

describe('Unfinished Feature Tests', () => {
  it.todo('should handle the new feature correctly');

  it('should handle an existing feature correctly', () => {
    // 実装済みのテスト
  });
});

vitest.dev

この例では、.todo を使ってテストをスキップし、プロダクトコードの実装が完了していないことを明示しています。
実装が完了した段階で、この .todo を削除し、テストを有効にします。

これにより、プロダクトコードとテストを分けて実装することができます。

また前述のフューチャトグルを使用している場合などに、以下のようにすることでプロダクトコードの実装が完了していないことを明示し、後で実装する際に見落としを防ぐこともできます。

describe('Unfinished Feature Tests', () => {
  it('should handle the new feature correctly', () ~> {
    expect(トグル をオンにすることでNot implemented エラーを吐く処理).not.toThrow();
  });
});

まとめ

今回、単一責任やテストなどの手法について例を用いてご紹介させていただきましたが、自分は過去とんでもなくレビューがしづらく、長期でマージがされない Pull Request を作成してきました、とっても。

そのことに課題を感じ、今回ご紹介した手法などを活用するようになり、レビューのしづらさを少しは改善する事ができました。

しかし、自分の Pull Request に対して良い変化を生じさせた要因は、これらの手法以上にレビュワーであるチームメンバーからのアドバイスが大きかったかと思います。

皆様も組織やチームによって、レビューの文化や方針など、それぞれ異なる部分も多いかと思います。

レビューのしやすい小さな Pull Request を作成するために、今回ご紹介させていただいた手法を参考にしていただきながらも、チームのメンバーに自分の Pull Request はレビューしやすいか、どういった点がレビューのしづらさを感じるかなどを聞いてみることをぜひオススメいたします。

お読みいただきありがとうございました！