リファクタリングの流れをコントロールするテクニック
はじめに
この記事では、巨大なレビューリクエスト、細切れのレビューリクエスト、それぞれで発生する問題を避けるために私が最近使っている、レビューリクエストのテクニックを紹介します。入念な準備が必要で、うまくいくとは限らない上級者向けのテクニックなので、私はここぞというときにしか使いません。
それではテクニックの解説の前に、解決したい問題の紹介からはじめます。
巨大なレビューリクエストはみづらい
これは開発者の多くが実感していると思います。レビュー箇所が膨大だと、
- 変更内容と意図を理解するだけで大変
- 見逃しが多くなる
という問題があり、さらに、
- レビューが大変なので、後回しにしがち
- レビューが始まっても「この箇所はOK、あの箇所はダメ！」と繰り返し、何日も経ってしまう
と言うことが起きがちで、その間にgitのmainブランチが他者のコミットによって先に進んで、マージするのも大変になっていきます。
細切れのレビューリクエストで途中Rejectもつらい
一方で何も考えずに細切れのレビューリクエストに分割しても、別の問題が生まれます。本来一連の流れとしてレビューすべき変更を細かく分割すると、途中のレビューリクエストで「この実装のやり方はダメ、もう一回全体をやり直すべき」と指摘される可能性が高まります。
こうなってしまうと途中までmainブランチに取り込んだ変更があるため、全体を巻きもどすのは非常に困難な作業になってしまいます。
なぜレビューリクエストは巨大になるのか？
そもそもレビューリクエストが巨大になるのは、理由があってそうなっているはずです。根本的な理由を理解せずに無理に分割したところで、解決策にはなりません。
例えば新機能開発の例を考えましょう。アプリケーションに全く新しい機能を加えるとき、その部分的な動作だけ正しくても、全体で間違った動作をしては意味がありません。そのため、
- 既存のアプリケーション全体を壊さない、小粒度の変更
よりも、
- 新機能の全体動作をおおよそ確認できる、大粒度の変更
を、ひとつのレビューリクエストとして送る開発者は多く、これは理にかなった判断です。
あるいは新機能開発以外の例として、大規模なリファクタリングを考えましょう。100ものHTTPエンドポイントをもつREST APIサーバーで、全てのエンドポイントについて統一された処理パターンを導入するとします。統一されたパターンとは、エラーハンドリングやセッション情報管理に関するものかもしれません。
こういった大規模なリファクタリングでは、以前の書き方がバラバラなことが多く、100のうち5つのエンドポイントだけ、どうしても統一パターンに書き直せない、ということが起きてしまうことがあります。これに最終段階でようやく気づいて、統一パターンそのものを書き換えなければならない、となったら大変です。
最初のレビューリクエストを送る前に、本当に実現可能なリファクタリングなのか検証すべきでしょう。そして、検証を終えたほぼ完成に近い変更は、巨大なレビューリクエストとなってしまいます。
上級者向けの解決テクニック
私が紹介するテクニックは、巨大なレビューリクエストによって得られる、全体の動作を確認できる安心感を保ちつつ、レビューリクエストを細かく分割してレビュワーの負担を減らす手法です。
まずは巨大レビューリクエストを送るときのように、自分の手元で全体像をほぼ完成させます。私の場合は数日から長くても数週間くらいの作業量でいったん区切り、それより長く数ヶ月かかる開発を自分の手元にため込むと、分割が不可能なほど巨大な変更になってしまうので避けます。
ここで書き換えたコードを、全部一度にレビューリクエストに出すのではなく分割するのですが、手元でコードを書いた時系列の順番通りに出さないことがコツです。
変更箇所の中で最初のレビューリクエストとして切り出すのは、
- 全体のうち中心的アイデアを反映した部分
- レビュワーと意見が分かれそうな箇所
です。一度完成形に近づいているので、これらは明確になっているはずです。こういった視点で切り出した一番最初のレビューリクエストがApproveされれば、そこで一番大きな山を越えてしまっているので、後の一連のレビューはスムーズです。
最初のレビューリクエストは全体を一度完成させて検証し終わっているので、自信を持って説明でき、レビュワーから来るであろう質問の多くにも的確に答えられるはずです。リクエストを提出するレビュイー側がしっかり準備して最初のレビューに望むことで、レビュワー側はApproveするだけ、ぐらいの状態にするが私の理想だと思っています。
もちろん、レビュワーから予想だにしなかった指摘がもらえるのは良いことです。その可能性は残しつつ、事前に検証と準備をできる限りはやって、レビュワーの負担を減らす、それによってレビュー開始以降の速度をあげる、というのがこのテクニックの意図です。
このテクニックの落とし穴
レビュー前に手元で完成形を目指している間に、自分のアイデアが間違っていることに気づいて、長期間の作業が無駄になってしまうかもしれません。あるいは、最初のレビューリクエストを出した時点であまりに議論が紛糾してしまい、結局マージしてもらえないかもしれません。
そういった長期の作業が無駄になるリスクを抱える点で、このテクニックは上級者向けのものです。また理解しやすい粒度と順番でレビューリクエストを分割する能力、全体を踏まえつつ情報の洪水を起こさない説明を添える能力も要求されます。
落とし穴があるとはいっても、そもそも上級者でなければ、何日も何週間もかかるような大規模な変更には手を出さないでしょうし、巨大なレビューリクエストを「ここまで書いてあるんだからマージするしかないか」と、後ろ向きな理由でマージしてしまうよりも、よほどましだと思っています。
まとめ
以上が私が紹介したかったテクニックです。私は毎回このやり方に従うわけではなく、ここぞというときにだけ、十分な時間をかけて使う方法です。全体がまとまってから狙いすましたようにレビューリクエストを出し、次から次に自分が思ったとおりにApproveされていくさまは、見ていて非常に気持ちがいいものです。
しかし、あくまで自分の高揚感のためではなく、レビュワーの負担を減らしつつ大規模な変更を実現するためのテクニックなので、目的を見失わずに使ってください。
