PRレビューを早くするために気を付けたい概要欄の記載

はじめに

プルリクエスト（PullRequest）レビュー時間に悩まされたことはないでしょうか。

• なかなかレビューしてもらえない
• レビューしたいが時間がかかる

なるべく早く終わったほうがどちらにとっても嬉しいですよね。
リクエスト側が記載する説明を工夫することで早くレビューを終わらせることができるかもしれないですよ。
このページでは、サンプルを元にどうすればよくなるかを説明してみようと思います。

※ここではリクエスト側がPRページで説明を書く欄を「概要欄」と呼びます。

サンプルの問題点

概要から何がしたいのかわからない

サンプル

# 概要
決済処理の不具合修正
# 変更点
- purchase_display テーブルにneed_englishカラムを追加

レビュアーが思うこと

# 概要
決済処理の不具合修正 `=> 何が起こっているのかわからん=>コードの正解が想像できない`
# 変更点
- purchase_display テーブルにneed_englishカラムを追加 `=> なんでそうなったのかわからん`

PRを読むときはレビュアーも作業前の実装者と同じ目線から始まります。
つまり下記を考えます。

• 何が起こっているのか
• 原因は何なのか
• どうすれば解決できそうか

これを概要欄で伝えきれなければコードレビューの開始が難しいのです。

PRリクエスト側が考えてほしい事

書く側が留意してほしいことは以下になります。

• 概要欄はコードを読むための道しるべ
• 読み手が理解できる粒度まで落とし込む

概要欄はコードを読むための道しるべ

PRを開いてまず目に入るのは概要です。
また、コードを読んで意図が読み取れなかった場合に立ち戻るのもこの概要欄です。
つまり、概要欄はレビュアーへの道しるべなのです。

コード読んで把握できないものの補間に使うものなので、コードに現れていない情報の重要度が高くなります。
サンプルへこの情報を追加してみます。

# 概要
## 事象
- 決済画面にて英語での表記が必須の項目があったが日本語が表示されることが発生していた。
## 原因
- ローカルマシンの言語設定が反映されているため
## 対応方針
- 強制的に英語表記にするためのカラムを追加することで表記言語の判定ができるように変更する
- レコードごとに指定を変える必要があるためカラムを追加する

# 変更点
- purchase_display テーブルに `need_english` カラムを追加 

経緯を説明する

「事象」「原因」がその記載になります。
Redmineや仕様書など別管理されているものがあればそちらのリンク、資料名を張るだけでも良いと思います。
とかく作業タスクの発生元とPRがリンクする記載があれば良いです。

サンプルの記載のままでは「エピローグしかない映画」のようなものなのです。
視聴者は見終わった後にパンフレットや関係者から情報を集めなければ理解できないのではないでしょうか。
その 情報収集がレビュー時間に含まれてしまう のです。

しかし、この情報があれば 自分が実装するならこうするといった方針が浮かぶ のではないでしょうか。
その方針と当該PRをすり合わせて一致すればLGTM。差異があればコメント。と比較的早めにレビュー作業が行えると思います。

余談ですが、
もしその方針がより良いものであれば、レビュアーは更に良い実装方法の提案をしてくれることでしょう。
誰しも重複コードや不要なデータの管理は避けたいですし、もっと楽な方法があれば採用したいと思います。
意図を理解してもらうためにしっかりと伝えれば、リクエスト側の想像を超えたレビューを受け取れる、してあげられると思っています。

コードの読み方を伝える

「対応方針」がその記載になります。
need_english を追加するに至った理由 を説明しようという意図になります。
「コードに定数を置けばいいのでは」などLGTMに至るまでの余計な横道をなくすためです。

サンプルの記載のままでは「推理小説で事件が起こった時点」のようなものです。
読み手は事件だけが見えていて、証拠や動機をこれから推理していこうとしてしまいます。
プログラムに話を戻すと、
１つの目的を達成するために多数のプロセスを経由しますが、コード上では個々のシンプルな処理しか目に見えません。
シンプルな処理がどうしてタスク達成につながるのかをレビュアーは推理することになります。
この 推理する時間がレビュー時間を膨らませてしまう のです。

推理するより犯人が答えを言ってしまえばすぐに解決するのですから、証拠や動機を記載してしまえばよいのです。
そうすればレビュアーはすぐにでも読み終えて次の推理小説を読みだすことでしょう（？）

読み手が理解できる粒度まで落とし込む

レビュー速度はレビュアーの「なんで？->なるほど！」の時間に比例して早くなると思っています。
これをリクエスト側の文書で加速させようと思います。

文書全般に言えることですが、留意すべきは 誰に理解してほしい文章か になります。
読み手の理解度によって粒度を調節します。

PRについては下記になります。

1. タスクの経緯を知っているか
2. 変更箇所のコードに理解があるか

それぞれの理解度ごとにどういった記載が必要でしょうか。

タスクの経緯を知っているか

経緯を説明する と同じ記載内容で意図も同じになります。
ただし、この記載がないことで時間がかかる ことが 本当に多い と思っているので特に留意してほしいと思っています。

変更箇所のコードに理解があるか

もし変更箇所のコードに慣れていない方がレビュアーとなったなら、処理フローの説明もあれば良いと思います。
基本的にレビュアー側で調べることが多いと思うのですが、全くわからない箇所のコード把握は横道を右往左往するため時間がかかってしまうと思います。
実装者側はある程度理解してコード変更を行っているはずなので、わかる範囲で説明を追加してあげれば時短につながると思います。

全く知らない人に丁寧に説明するなら下記のように箇条書きの文章で説明を入れれば理解が早いと思います。

# 概要
~~省略~~
### 処理フロー
1. Web画面から purchase_api がコールされます。
1. createReponse() にて表示文章のレスポンスが生成されてることになります。
1. get_description() にて既存の言語判定が行われており、need_~~カラムをチェックしています。
    - 言語カラム英語を足すことでチェック処理に含まれるようになります。
## 対応方針
~~省略~~

ある程度理解のある人であれば下記のように関数の流れだけでも良いかもしれません。

# 概要
~~省略~~
### 処理フロー
- purchase_api() 決済APIのエントリー
    - createReponse() レスポンスの生成
       - get_description() 文書取得に言語判定を追加する
## 対応方針
~~省略~~

サンプルは単純なものですが、データの流れが複雑であればあるほど細かい粒度で説明が必要になると思います。
複雑なデータフローで不具合に至るなどコードだけでは見えないケースは特に必要だと感じています。
この記載がレビュアーの コードに対する「なんで？->なるほど！」 を短くします。

最後に

ここまで書いたことはリクエスト側の負担が大きくなることだと思います。
しかし、レビュアーが理解するために必要なアクションより、リクエスターが知っていることを記述する方が負担が少ない のではないでしょうか。
「リクエスト側が知っていること」と「レビュアーがしらないこと」の差は思っているより大きいと日々感じています。

レビューが早くなれば、その分マージも速くなり、結果的にメリットは返ってくると思います。
少しの手間で誰かの時間が増えてくれればいいなと願っています。