yigarashiのブログ

学んだことや考えていることを書きます

プルリクエストが大きくなると判断が歪むことに気付いた

僕の個人的な話で、一般の話ではありません。もし心当たりがあれば一緒に気付きましょう。

とある日、大きめの実装タスクがアサインされました。大きめというのは実営業日で1週間ちょっとかかるイメージです。内容はそこまで難しくなく、なんとなく書いて大きめのプルリクで済ませてもシュッと進みそうな印象でした。ということで、+500行くらいのプルリクと+400行くらいのプルリクに分けて実装を進めたわけですが、その中で一部のテストをどのくらい丁寧に書くか迷いました。レガシー寄りのシステムで堅牢さはそこまで要求されていないが、自分が安心できるくらいのテストは書きたい、しかし丁寧にテストするにはちょっとしたヘルパメソッドを用意する必要があるだろう、という状況でした。僕の最初の判断は次の通りです。

まず丁寧なテストはスキップすることにしました。代わりにヘルパメソッドを実装しなくて済むようモックを使った簡易的なテストを行うことにしました。手元のブランチには既にdiffが積み上がっており、さらにdiffを大きくする変更は、得られる安心に対して割に合わないように思ったからです。

出来上がったプルリクをレビューしてもらって、特に問題はありませんでした。ただ、修正の必要はない程度のコメントとして、件のテストの箇所に「ここはモックするんですね(意訳)」というコメントをもらいました。まさしくそこは迷ったし、他の部分との一貫性もないので、至極真っ当な疑問だと思いました。このコメントを受けた僕の判断は次の通りです。

やはり丁寧なテストをすることにしました。ヘルパメソッドを実装して、モックを使っていたテストを置き換えました。実装してみたらたかだか数十行で、動作確認を含めても10分で終わりました。追加のコミットを見てもらうだけなので何も躊躇することはありません。

ここまできて、自分の判断がプルリクエストの大きさによって歪んでいたことに気がつきました。個別に見たら簡単に取り組めるはずの事柄が、大きいプルリクエストに含まれることによって阻まれてしまっていたのです。原因はdiffの大きさというパラメータがコストパフォーマンスの判断に入り込んでしまったことでしょう。

この気付きを受けてなんらか改善を試みます。とはいえ、一番困るのはこうした誤認が起こりうることに気づかずに方針を縮退させてしまうことで、気づいた今となっては恐るるに足りません。一番良いのは、上手にタスクを分割してそもそもこういう誤認が起こりづらい状況を作ることです。当たり前ですね。とはいえ、それが過剰であったり面倒であったりするのが現実なわけです。落穂拾いなどと称してわざわざ別のプルリクエストにするほどの重要度はないが、その場でやっておけると世界が少し良くなるような事柄が無数にあります。そこでどうするか。diffの大きさを気にしすぎないというのはひとつの手だと思います。実際、もともと大きめのプルリクエストに数十行のコミットが足されたところで、レビュワーの負担はほとんど変わりません。diffが大きいのは悪というプラクティスに引っ張られて良い行いをやめるくらいなら、開き直って実装するほうが前向きではあります。

じゃあ200行も300行も足して良いかというとそうではないので、いつもの通り要はバランスという結論になるのですが、とにかく、diffの大きさに自分の判断が振り回されていないか考えてみてはいかがでしょうかという話でした。