効率的・効果的なプルリクエストのための取り組み

本記事は【プルリクウィーク】 1日目の記事です。
💻 告知記事 ▶▶ 本記事 ▶▶ 2日目 📚

こんにちは、フロントエンド領域を中心に活動しているシステムエンジニアの山田です。
昨今のシステム開発においてはGitを使用することがほとんどかと思います。

また、開発プロセスとしてプルリクエスト（以下、PR）を利用するチームも多いのではないでしょうか？
そこで、この記事では実際に私が行なってきたPRを効率的にレビューするための取り組みと、PRを利用する際に意識した取り組みについてご紹介しようと思います。
※ここではTypeScriptを利用した開発経験をもとに記載します。

システム的な取り組み
開発者（レビューイ）としての取り組み
レビュアーとしての取り組み
おわりに

システム的な取り組み

フォーマッターを利用する

まずはPrettierのようなフォーマッターを導入することです。
コードの可読性という点でのメリットがありますが、PRにおけるメリットも存在しています。

共通のコードフォーマットがチームで存在していない場合、インデントの違いやスペースの有無、文末のセミコロンの有無など開発者によって違いが出てくる可能性があります。
そういった違いを放置していると、意図せぬタイミングで差分としてPRに上がってくることがあります。

PRのレビュアーとしては、コードの差分を確認することが多いと思います。
PRの本分は機能開発や不具合修正にあたるため、差分についてもフォーマットの違いのような余計な情報は現れてほしくありません。
そういったノイズを排除するだけでもレビューする側の負担は下がるかと思います。

静的解析を利用する

次はESLintのような静的解析（Lint）の導入です。
静的解析はソースコードを解析して、指定したコーディングルールにしたがってソースコードが書かれているかをチェックしてくれます。

チームで開発する際はコーディングルールに従って開発することも多いのではないでしょうか？
そういったルールはなるべく、先程のPrettierやESLintに任せてしまうようにしています。

繰り返しの内容になりますが、レビュアーとしては機能開発や不具合修正のレビューに集中できることが望ましいです。
システム的にチェックできる部分はどんどんシステムに任せていくようにしています。

テストコードを書く、実行する

最後にテストコードについてです（開発者が取り組むべきポイントでもありますが）。
TypeScriptではJestやVitestあたりがユニットテストでは有名どころでしょうか。

テストコードの重要性などについてここでは触れませんが、追加すべきテストコードは追加し、既存のテストコードは実行するようにチームとして取り組んできました。

PRは関所としての役割も持っている場合もあるかと思います。
例えば、共通的な処理に修正を入れたところ、ある機能がデグレードした、という経験はありませんか？
レビューのタイミングで気がつけるのがよいのですが、人間ならば見落とす可能性はありますよね。

テストコードを実行することで、こういったケースは防げることもあるかと思います。
また、新規に開発した部分に関してテストコードを書くことで、品質の確保と上記のような事象の予防を見込めます。

この辺りのフォーマットのチェックや静的解析、ユニットテストはCIとして定義することが多いです。

その他

TypeScriptに限った話ですが、tsconfigのstrictはtrueに設定するようにしています。
strictがfalseなプロジェクトを引き継いだときは、コーディング用のtsconfigとビルド用のtsconfigを分けるなどの工夫をするようにしています。

開発者（レビューイ）としての取り組み

PRの範囲に注意する

私のチームでは課題管理システムを利用することが多いですが、みなさまはいかがでしょうか？
利用している場合は、課題（チケット）に紐づいてPRを作成することが多いと思います。

親切な方に多い気がしますが、複数の課題をまとめて1つのPRにすることがあります。
レビュアーの負担軽減を意識しての行動かと思いますが、私は、基本的に課題とPRは1対1になるようにしています。
※例えば不具合において、原因が同じ場合はその旨を課題に記載して課題側をクローズするようにしています。

個人差があると思いますが、私としては内容が複合的な1つのPRより内容が単純な複数のPRの方がレビューしやすいです。
対応内容と差分が明快であるほど効率的なPRのレビューが可能かと思います。

なので、課題の分割粒度も重要になってきます。

PRのコメント機能を活用する

しかしながら、どうしても対応内容が複雑になってしまうことはあるかと思います。
そのような場合は、開発者（PRの作成者）が解説のためのコメントをいれることもやってきました。

レビュアーとしてはインプットの情報が乏しければ乏しいほど時間もかかってしまいますので、私がPRを作成するときは効率化のために取り組むようにしています。

コミットに意味をもたせる

PRの規模が大きい場合、コミット単位で差分を確認するレビュアーもいるのではないでしょうか？
先述の通り、課題化のタイミングで細分化が好ましいですが、それでも規模が大きくなることもあります。
そういった場合には私もコミット単位で確認することが多いです。

私が開発者としてコミットする場合は「◯月◯日の作業」ではなく「A機能に◯◯の考慮を追加した」のように内容が明確になるようにしています。
後々、過去のコミットログを確認するときにも役立つはずです。
このあたりのコミットルールはチームで決めて運用するのがよいと思います。

また、PR作成前にgit rebase -iでコミットログを整理することも多いです。
デバッグコードを消し忘れていたときなどはこの方法で修正し、無駄なコミットログが生まれないようにしています。

レビュアーとしての取り組み

ナレッジ共有の場とする

PRは開発者がレビュアーにレビューしてもらうという場ですが、有益な情報はチームで共有するように取り組んでいます。
PRの1つ1つのコメントはナレッジであると考えており、これらを開発者とレビュアーだけに閉じ込めてしまうのはもったいないかと思います。

また、チーム開発ではSlackなどのチャットツールを利用していることも多いと思います。
詳細なやり取りなどはチャットで済ませてしまうこともあるかと思いますが、経緯の概要や決定事項についてはPRのコメントに残すようにしています。

関係のないPRを見る機会はないというチームもあるとは思いますが、簡単なナレッジ共有の場として活用してみてもよいのではないでしょうか？
コードレベルでナレッジを共有するには、PRは使い勝手がよいと思います。

ウォークスルー形式でレビューをする

開発者が主体となってPRの内容を説明し、レビューする方法です。
過去実施していた際、参加者はレビュアーのみならず他の開発者も対象としていました。

レビューの時間だけ参加者を拘束することになり、効率面では正直なところよいとは言えません。
使い所は慎重にしていました。

例えば、プロジェクトが立ち上がって最初のある一定の期間のPRはこのような形でレビューしていました。
スタートのタイミングで、ある一定の品質確保のためには非常によかったと思っています。
※私個人の感覚として開発が進んでいくと既存のソースコードを模倣する傾向があると感じているため、立ち上がりは重要だと考えています。

他にもコアな部分の開発であったり、難易度の高い部分の開発などで活用するようにしていました。

また、指摘事項や決定事項は通常のPRと同じようにコメントとして残すようにしていました。
これは先ほどのナレッジ共有の場とする、という観点からそうしていました。