はじめに
こんにちは。進捗ゼミです。
チーム開発では、複数の開発者がGitとGitHubを使って連携し、プルリクエストを通じてコードレビューを行うことが多いです。
一般的に、プルリクエストは小さいほうがチームメンバーのレビューが簡単になり、バグが検出されやすくなり、フィードバックが受けやすくなると言われています。プルリクエストを小さくするメリットについては以下の記事で分かりやすく書かれています。
変更単位を小さくすることのメリットが知られているにもかかわらず、プルリクエストは気を抜くとすぐに大きくなってしまいます。
この記事では、「気をつける」以上の具体的な対策を説明します。
対策の方針
まず、ソフトウェアエンジニアがどのような手順でプルリクエストを作成しているかを考えてみましょう。
- 問題の特定：与えられたタスクに対して、現在の状態とあるべき姿を調査し、問題の本質を理解します
- 解決策の決定：問題を具体的に解決する方法を比較検討し、最適なものを決定します
- タスクの分割：解決策を実現するためのコードの変更単位を決定します
- 実装：作ります
- プルリクエストの作成：コードの変更とそれに対する説明を行い、チームメンバーと議論します
このステップのそれぞれで、プルリクエストを小さくするための具体的な方策を考えることができます。
ここでは、それぞれのステップでの対処方針について軽く説明します。
問題の特定
このフェーズでは、特にどんな文書も作成されません。単に開発者のメンタルモデルが整理されるだけです。
ここをスリム化するのは誤りだと考えています。
解決策の決定
このフェーズでは、問題解決のための方針を決定します。この方針によってプルリクエストのレビューしやすさの上限が決定します。本質的に難しい解決策を使うと、どれだけ分かりやすく説明しようが、レビューは難しくなり、プルリクエストは肥大化します。
そのため、正しい解決策を選択する際に、できる限り小さくするための方法を検討します。
タスクの分割
このフェーズでは、具体的な実装の手順を決定します。この分割が失敗すると、プルリクエストは肥大化してしまいます。
そのため、レビューが容易な単位まで分割する技法を検討します。
実装
このフェーズでは、プログラミングを行います。
分かりやすく、読みやすいコードを実装する方法については、すでにたくさんの先行研究があるため、この記事では説明しません。
プルリクエストの作成
このフェーズでは、変更単位をレビュアーに対して説明し、実際にレビューを行います。
この段階まで来てしまうと、レビュイーにできることは少ないです。しかし、ある程度の努力によって、レビューを容易にすることが可能です。
解くべき問題、正しい解き方
ソフトウェア開発で最も大切なこと
ソフトウェア開発の困難は、たいていの場合、ソフトウェアが人間の理解できる範囲を超えて複雑であることから来ています。今回のテーマであるプルリクエストの肥大化についても同様です。そのため、ソフトウェア開発の基本原理は次のようなものです。
ソフトウェア開発の最優先事項
- 開発者はできる限りすべてを理解し、多くを記憶し、有能であること
- ソフトウェアはできる限り問題を抽象化し、知識を局所化し、誰でも理解・修正できるものであること
よりスローガン風に言えば、作る人は賢く、作るものはバカでも分かるように、です。
問題の特定をサボるな
問題の特定ステップでは、開発者が問題の原因を調査し、タスクの結果どのような状態になるのかが理想的かを理解し、そのためにどのような解決策をとることができるかをリストアップします。
このステップは、ソフトウェア開発の最優先事項で説明した「作る人は賢く」の部分に相当します。
このことから、次の重要な原則が分かります。
レビューの簡素化を言い訳にして問題の本質の理解を怠ってはならない
最終的にどのような解決策を選ぶかに関わらず、解決策の手札をたくさん持っているのは重要です。このフェーズをおろそかにすると、簡単だと思っていた解決策が後から大変だと気づいたり、より有効な解決策を見落としたりしてしまうからです。
正しい解き方を見つける
速い解き方と正しい解き方
一口に解決策と言っても、様々なものがあります。プルリクエストの大きさについてのもっとも基本的な性質が、スピードと品質です。
- スピード：どの程度簡素な変更で問題を解決できるか
- 品質：その解き方がどれぐらい問題を根本的に解決するか
基本的に、両者はトレードオフになっています。
品質の高い解決策は、レビューが困難で、プルリクエストも大きくなりがちです。しかし、適切に品質を高めていかないと、ソフトウェアは複雑化していきます。
あえてやらない美徳
プルリクエストが肥大化する原因のひとつとして、開発者は品質が高い選択肢を知っている場合、それを実現したくなることがあります。
これは、開発者の自然な心理が原因です。例えば、次のような心理が働きます。
- 正しい姿を見つけるには、スピード重視の解決策よりも多くの努力が必要であるため、サンクコストの意識が生まれる
- 品質が高い選択肢を知っていると、スピードの高い解決策の欠陥が明確に理解できる
- 正しいと自信を持って言えるものを作るほうが楽しいし、後ろめたさがない
- よく勉強している開発者ほど、ハンマーを持っているとすべてが釘に見える現象に引っかかりやすい
手札を見つけるフェーズでは、勉強しているフェーズでは、自分の技術力を高めるために、自分の能力の100%を使って品質が高い答えを見つけることを要求されます。しかし、チーム開発で開発者として価値を出そうとすると、途端に「正しい」解決策を選択することが、「正しくない」と言われ始めるのです。
能力があっても、価値を発揮できているとは限らない
- 開発者の能力は、最も複雑な（たいてい最も品質が高い）手札によって決まる
- 開発者の価値は、選んだ手札の良さによって決まる
筆者が先日読んだブログ記事にも似たような考えが書かれていました。
それでは、チーム開発においての良さはどのように決まるのでしょうか？
それはより人間的で、複雑で、ビジネスに近い要因によって決まります。
チーム開発における手札の良さ
手札の良さは、次の要因などによって影響される。有能な開発者は、これらを総合して、チームにとって最も価値のある選択をする。
- 対処すべき問題の緊急性
- 実装量
- 理解の容易さ
- メンバーの技術的な習熟度
- 技術的な負債を残した場合のコスト
- 対象となる領域の重要性
- 対象となる領域の変更頻度
非常に多くの問題が複雑に絡み合っているため、この選択のスキルを学ぶのは難しいです。
そもそも、手札の良さを判断する土俵に立つには、たくさんの手札を持っていないと始まらないため、前提条件すら満たせない場合もあるでしょう。
しかし、常に的確な判断ができなくても、自分なりに考えて、あえて技術的に正しくない選択をすることができれば、エンジニアとして次のステップに進めたと言えるはずです。
本質的難易度
品質が高い解決策を選んでしまう心理的な要因として、次のような心理があります。
言葉を尽くして分かりやすく説明すれば、複雑な問題でも理解可能になる
残念ながら、これは正しくありません。正確には、解決策の難しさは、固有の本質的な複雑さと説明に起因する偶発的な難しさから構成されています。
-
本質的な複雑さ：その解決策自体がどのぐらいの認知負荷を持っているか
- 関連するコンポーネントが多い場合、複雑性が高い
- 採用しているアルゴリズムが難しい場合、複雑性が高い
-
偶発的な複雑さ：説明の文章がどの程度の認知負荷を持っているか
- 具体例が全くない場合、複雑性が高い
- 段落構成が破綻している場合、複雑性が高い
より分かりやすく言えば、難しいものはどれだけ言葉を尽くしても難しいのです。
詳しい説明はある程度複雑性を緩和するが、限界がある。
ときには、解決策自体を変更しなければ必要な単純さが得られないときもあるのです。
問題を分割せよ！
正しい解決策を見つけたら、次は問題を分割しなければなりません。
残念ながら、問題の分割は、多くの場合困難です。ソフトウェアは複雑に依存しあい、論理的に分割可能な粒度はレビューしやすい粒度よりも圧倒的に大きい場合が多いです。
この不都合な現実に対抗するため、様々な分割手法があります。
垂直方向の分割
垂直方向の分割とは、単一のタスクに対して、マージまでに何度もレビューをしてもらう分割方式です。ある程度巨大な変更でも、変更が大きくなる過程で複数回レビューして開発者全員の理解度を高めることで、レビューの精度の低下を緩和することができます。
報連相の極意
プルリクエストが肥大化することの根本的な問題点は、チームメンバーに意図が理解されないことです。
そのため、肥大化していても、適切に報連相をしておけば、最低限誰にも理解できないコードになることは避けられます。
事前にドキュメントを書く
コードで説明しにくい抽象的、基礎的な事項は、コードを書く前に自然言語で説明してある程度レビューを受けておくとよいです。
- コードを読むための事前知識を文書にしておく
- 変更の基本コンセプトを図表で説明する
- 設計の理由を文書化しておく
設計の理由の文書化というテーマでは、ADRという考え方があるので、紹介しておきます。
これは、報連相における、事前の相談に相当します。
PoCを書く
自然言語を使う以外にも、コードを使って基礎的な方針を説明することもできます。
その方法は、簡易的な実装内容をざっくり書いて、動作するコードで説明することです。
いわゆるPoC（概念検証）というものです。
事前にPoCをレビューしてもらうことは、レビューの負担軽減にとって有用です。なぜなら、レビューの困難のうち、変更内容の理解の部分を大幅に減らすことができるからです。
レビュアーは、未知のコードをレビューするとき、まずコードがどのような意図で書かれているかを理解して、そのうえでコードの問題点を探します。したがって、コードの意図が不明だと理解のために脳のリソースの大半を消費してしまいます。これでは、潜在的なバグの発見はおぼつきません。
対して、PoCのレビューはコードの品質やバグについてはあまり気にせず、コードの意図する部分だけを理解してもらえばよいです。
これにより、本番の変更のレビューでは、コードの品質向上の指摘に集中することができます。
三段階分割法
ある程度の方針を共有した後は、本実装を行います。本実装においても、ある程度の垂直分割が可能です。ここでは、三段階に分割する手法を紹介します。
0から1
0から1のフェーズでは、現在のプルリクエストで行う予定の変更の典型例をひとつだけ列挙します。
プルリクエストでは、基本的な変更と、それに付随する軽微な変更から構成されることが多いです。そのため、最もレビューが必要な部分だけを作った段階でレビューを受けることで、より本質的な理解を促進できます。
1から9
1から9のフェーズでは、0から1で行った変更と同様のパターンの変更をひたすら行います。
変更の量は多いですが、変更内容はすでにレビューされたものがパターン化されているだけなので、量に比べてレビューが容易です。
9から10
9から10のフェーズでは、典型的なパターンでは捉えきれなかった軽微な修正を行います。
このフェーズの分量が多くなる場合は、プルリクエストの関心事が多いため、分割したほうがいい可能性があります。
水平方向への分割
水平方向への分割は、解決策を複数のマージ単位に分割する方式です。いわゆるサブタスクへの分割がこれにあたります。垂直方向の分割に比べると、タスクの依存関係を考慮して小さな単位でマージしていく必要があるため、分割が難しいです。しかし、水平方向への分割の方がマージ単位を小さくできるため望ましいです。
キレイに分けられるタスクは絶対に分けよう
水平分割についての最も基本的な原則は、水平分割が完璧にできるタスクは必ず分割するということです。すでに説明したように、コードの依存関係などの理由で、水平分割は常に可能なわけではありません。そのため、水平分割ができる場面は貴重なのです。
中間状態を作ろう
古い仕組みを新しい仕組みで書き換えていくときに、一度にすべてを変更しようとすると、変更が大きくなります。
このような場合、移行途中で古い仕組みと新しい仕組みが混在している状態を一時的に作り出すことで、水平分割が可能になる場合があります。
これは、ライブラリの移行を考えると分かりやすいです。多くの場合、バージョン1のライブラリは、バージョン2のライブラリが公開されて十分な移行期間を経てから廃止されます。
プルリクエストでも同じように、修正後の新しい仕組みを全員が使っていない、不完全な混在の状況を許容するべきなのです。
もちろん、正しいソフトウェアを作るという観点からすると、中間状態がビルドされて本番環境で動作しているというのは気分がいいものではありません。
なるべく早く（できれば、関係者の記憶が残っているうちに）中間状態を廃止できるように優先度を上げて開発を行うべきです。
「ついでのリファクタ」をしない
機能を追加していると、リファクタリングが可能な場所をたくさん見つけることになるでしょう。
リファクタリングを行うこと自体は悪くありません。しかし、「ついでに」リファクタリングすることは、プルリクエストの理解の難易度を大幅に上昇させてしまいます。
直したい内容は手元にメモしておいて、マージされてから個別に直すべきです。
完成図を描いてから分割する
少し強引で力業な手法ですが、一度手元でレビューができないぐらい一気に変更をしてしまい、完成したコードを見た後に、適切な分割方法を考えることができます。
この手法は、最初の実装に大きな時間がかかるものの、修正完了後の動作する完成品があるので、適切な水平分割のイメージが湧きやすいです。また、完成したコードがあれば、基本的にはGitの操作と少しの手作業で水平分割ができるので、完成からプルリクエストの作成までの手間は小さいです。
プルリクエストを説明しよう
プルリクエストでどんな変更をしているかを説明することで、ある程度レビュアーの負担を軽減することができます。とはいえ、あくまで小手先のテクニックなので、問題の分割の失敗を取り返せるほどではないです。
重点的にレビューするべき場所を説明する
難しいファイルと簡単なファイルがある場合、事前に説明しておくとレビュアーのリソースを節約できます。
プルリクエストのコメントのテンプレートにしたがう
大抵の場合、プルリクエストでは説明してほしいことが書かれているはずです。それらのテンプレートにしたがって書いておいた方が認知負荷が小さいです。
ややこしい場所は事前にコメントを使って説明する
コードの特定の場所がややこしい、といったような場合、コードコメントやレビューコメントを使って説明しておきましょう。
プログラムを読むときのややこしさならコードコメント、レビュー時に発生するややこしさ（中間状態に起因するものなど）はレビューコメントを使うとよいでしょう。
おわりに
この記事では、プルリクエストが大きくなってしまう原因から対処法まで説明しました。
冒頭では単に漠然と「気をつける」は対処法にならないと説明しましたが、これらの手法を理解し、自然に使いこなせるようになるまでは、絶えず意識的にプルリクエストの大きさを意識することが必要です。
レビュアーにやさしいプルリクエストを作って、よいチーム開発を目指しましょう！