Pull Request、ちゃんとpullしてますか？

記事を読んでいただきありがとうございます。
普段はPM業務をしている駆け出しエンジニアです。

ある時先輩に、Pull Request（以下PR）レビューのコツについて聞いてみると、その先輩はレビューをする時はいつもgit pullをしてlocalで動作の確認をしているそうです。その話を聞いて面白いなと思ったので、記事にまとめてみました。

PRとは、開発の内容（マージ先との差分）を確認してもらうために提出する、Githubの機能の1つです。
チーム開発をした方であれば、誰もが使ったことがあると思いますが、割とGithubの画面上だけの確認になってしまいがちです。よくよく考えてみれば、「(git) pull依頼」という意味なので、元はgit pullをする前提だったのかなと思います（常識だったらすみません）。

エディタの静的エラーが出る

今回その先輩と一緒に入っている案件は、PHP/Laravelの案件でした。
PHPは動的な言語なので、何も設定していなければ「動かしてみるまでエラーがわからない」です。

例えば、2つの引数が必要なメソッドに1つだけしか渡していない状態でも、実際に動くまではエラーになりません。

function test(int a, int b): int
{
  return a + b;
}

// 実際に動かすまでエラーが発生しない！
test(1);

こういったミスがあったとしても、CIを導入していない限りGithub上では分かりにくいです。
レビュワーも人間ですから、確認漏れが発生してしまう可能性もあります。

そこでgit pullをしてみましょう。エディタの静的解析ツールが即座に警告を提供するため、動的言語でも事前にエラーを検出できます。
上記だけではなく、未使用の変数や型エラーなどもいけます。

function add(int a, int b): int
{
  return a + b;
}

// 波線で警告してくれる！
add(1);

もちろん、CIによる自動テストが最も確実ですが、git pullを使うことで発生するエラーを前もって防げるのは大きな利点です。
上記以外にも、型エラーなども出してくれるので便利です。

スペルミスが防げる

エディタやその拡張機能によっては、英語のスペルミスを指摘する機能があります。
スペルミスはCIテストでは検出しにくく、人による確認でも見落とされやすいです。

function retrunText(): string
{
    return "Hello, World!";
}

// スペルミスでもエラーが発生しない
$str = retrunText();

上記のretrunText()は見ての通りスペルを大ミスしています。
にもかかわらず、エラーにはなりません。正しく間違ったスペルのメソッドを呼び出しているからです。

正しくは以下ですね（命名がキモいのは一旦置いておいてください）。

function returnText(): string
{
    return "Hello, World!";
}

$str = returnText();

スペルチェック機能を備えたエディタなら、この種のスペルミスも検出します。

自動整形が効く

これも大きいメリットです。
不要なスペース、インデント、use宣言の並び替え、etc....
数多くの小さな問題を一発で検出できます。

huskyなどのツールで自動整形が設定されている場合を除き、GitHubのfile changesでこれらを全て検出するのは困難です。

ファイルをセーブしただけで一撃で自動整形し、remoteとの差分を見ればどこを指摘すれば良いのか丸わかりになります。わざわざ一行ずつ「不要な改行はないか...？ズレてるインデントは...」と探す必要もないのです。

実際に動かせる

検証環境の前に実際に動かすことができるのも大きなメリットだと思います。
個人的にはCIでテストコードが自動化されていても心配になってしまうので...。

localで手順に従って動作確認を行うことで、ドメイン知識に基づいた具体的なフィードバックが可能になります。
他の人のタスクに関するドメイン知識はふわっと理解している場合が多く、コードの清潔さに焦点が当たりがちです。

一番大切なのは要件を満たしているかどうかなので、テストコードだけで不安な場合は、local環境での実行をお勧めします。

逆にデメリット

逆にデメリットだなぁと思った部分で言うと、当然ながらGithub上のみでレビューするよりも時間がかかります。
さらに、各開発者の環境によっては、localでのセットアップや依存関係の問題で実際に動作確認するまでに追加の手順が必要になる場合があります。

ただ、もしCIが導入されていない現場だった場合、動作検証などには時間をかけるべきと思うので、localにpullして確認することも検討してみてはいかがでしょうか？