レビューアー（私）がうれしいレビューリクエストの書き方

7400 ワード
MergeRequest pullrequest レビュー レビュー テキストリンク

今年も始まりました。ラクスアドベントカレンダー 1日目です！

初日から長く＆説教くさくなり過ぎたなと反省しつつも、これ以上こだわると間に合わなさそうなので公開します

はじめに

この記事では、コードレビューをすることが多い自分の経験をもとに、「こういうレビューリクエスト（GitLabのMerge Request、GitHubのPull Request）だと助かる」というポイントをまとめます。一緒に仕事をするメンバーが変わったときに、レビューアー 1 としての自分の考えがまとまっている資料がほしかったという下心があります。

なお、ここでは レビューリクエストの書き方 にフォーカスしています。その前提となるコードの良し悪しや、コミットの粒度、コミットコメントの書き方などはスコープではありません。また、ラクスではバージョン管理にGitLabを採用しているため、多少GitLabに寄った説明になっている部分もありますので、ご注意ください。

1. レビュー条件を満たそう

いきなり書き方と関係ないとツッコまれそうですが、わりと軽視されがちなので改めて確認しておきます。レビューリクエストというのは、レビューイーからレビューアーへのタスクの受け渡しです。レビューに出す条件はチームごとに決まっていると思いますが、そもそもの前提条件を満たしていなければ書き方以前の話で手戻りを発生させてしまうことになるので、お互いの時間を無駄にしないよう気を付けましょう。よくあるルールとしては、

  • Asigneeを設定する
    • タスクのボールを誰がもっているか明確にするために、レビューアーを設定しましょう。レビューアーを事前に決めないチームの場合も、レビューアーが決まったタイミングで設定します。差し戻し後の再レビューのときは特に忘れがち
  • WIPを外す
    • GitLab では作業中（未完了）のMerge Requestを WIP とマークし、誤ってマージしてしまうことを防ぐ機能があります。レビューに出すときはマージしてもらいたいわけですから、 WIP を外し忘れないよう注意しましょう
  • CIを通す
    • CIに通っていないレビューリクエストなど論外です。信頼貯金が減るだけなのでやめましょう

2. 必要十分な情報を伝えよう

レビューリクエストは、読ませるための文書ではなく、情報を簡潔に伝えるための文書です。長すぎず短すぎず、レビューアーに必要な情報を、もっと言うとレビューアーが必要な情報だけを、絞りに絞って伝えましょう。「お忙しいところ申し訳ありませんが、レビューお願いします」的なやつは要らないです。

  • タイトル
    • もっとも重要な情報です。変更の概要を1行で表現します。UI調整 とか コード指摘の修正 とかは何かを言っているようで何も言っていないので、登録ボタンの位置をツールバーに移動 のように具体的なものにするとよいです
  • 内容
    • レビューリクエストの説明に何を書くかは、プロジェクトや修正の種類によって変わると思いますが、レビューアーの知識・コンテキストに合わせて説明するのが原則です。修正内容の説明ばかりに注力しがちですが、むしろその修正が必要な背景や既存機能への影響を説明してもらえるとレビューアーは安心です
    • 機能追加の場合、その機能が必要になった背景 や 修正点、既存機能への影響、動作確認手順 などを説明します。コードから読み取れる修正点よりも、その機能が必要な理由や解決したい問題などの背景を説明してもらえるとレビューしやすいです
    • バグ修正の場合、不具合の原因 や 修正内容、同じ不具合のほかの発生箇所、動作確認手順 などを説明します。不具合が発生した箇所だけでなく、ほかに同じ不具合が発生する箇所がないかまで説明してもらえるとレビューしやすいです

GitLab/GitHub には Merge Request/Pull Request を書く際のテンプレートを登録しておく機能があります。プロジェクトで決まったフォーマットがある場合や、必要な情報の書き漏れが多発するようであれば、テンプレートを活用するとよいでしょう。

3. 画面キャプチャを活用しよう

「百聞は一見にしかず」ということわざにもある通り、文章だけで説明しようとするのではなく、画面キャプチャを積極的に使いましょう。UIを変更するような修正の場合は、特に有効です。理由はいくつかありますが、

  • レビューアーはコードの前に外部仕様として画面を先にレビューできた方が無駄がない
  • レビューアーは自分が作ったわけではないので、画面があるとレビューイーとの前提知識のギャップを埋めやすい

意外に知られていない気がしますが、IssueやMerge Requestにはクリップボードから直接画像を添付できます。スクリーンショット（静止画）だけでなく、スクリーンキャスト（動画）も撮れるようにしておくと、動きのある機能を見せたい場合に重宝します。

私が愛用しているアプリ（macOS）は次の通りです。

  • Skitch - スクリーンショット用。手軽に図形や文字を書き込める点がお気に入り
  • LICEcap 2 - スクリーンキャスト用。古くさいけどシンプルで余計な機能がないのがお気に入り

4. 箇条書きやチェックリストを活用しよう

改行や全角スペース、■ □ ○ ・ などの記号を駆使して独自スタイルで文章を構成するのではなく、Markdownを使って文章を構成しましょう。文章で長々と説明するよりも、積極的に箇条書きを使った方が見た目にも分かりやすく、伝わりやすい説明になります。

  • 箇条書き
    • 修正内容や影響範囲の列挙など、類似の情報を複数提示する場合には、 * で箇条書きにしておくと情報の粒度がひと目で分かりやすくなります
  • 番号付き箇条書き
    • 再現手順や動作確認手順のように、連続する操作を説明する場合は、1. で番号付き箇条書きにしておくと順番がひと目で分かりやすくなります。2行目以降も 1. にしておけば自動で採番されます
  • チェックリスト
    • 動作確認などレビューアーにチェックしてもらいたいことが複数ある場合は、[ ] でチェックリストにしておくと、確認が完了したものからON/OFFできて親切です

例えば、次のような動作確認手順を作ると、レビューアーは文章で説明されるよりも、手順と確認事項の把握がしやすくなると思います。

### 確認手順

1. ログイン画面を開く
1. 誤ったログイン情報を入力してログインボタンを押す
    * [x] 認証エラーになること
    * [ ] ユーザー名とパスワード欄がクリアされること
1. 正しいログイン情報を入力してログインボタンを押す 
    * [ ] ホーム画面が表示されること

5. コードのインラインコメントを活用しよう

ソースコードに対するレビューコメントはレビューイーも追加できるため、あらかじめレビューアーに知らせておきたいことがある場合は利用しましょう。レビューリクエストの内容欄で特記事項や補足として説明することもできますが、特定の実装箇所に関する補足であればインラインコメントの方が伝わりやすいです。

ただし、ソースコードに対する特記事項があるのであれば、そもそもソースコード中のコメントとしてコミットしておいた方がよい場合もあります。単にレビューの過程でレビューアーだけに知っておいてほしいのか、今後コードをさわるほかの開発者にも注意してもらいたいのかを考えて判断するとよいでしょう。

6. チームがやりやすいようにしよう

ここまで言っておいてあれですが、最終的には自分たちのチームがやりやすいようにしましょう。メンバーのコンテキストが揃っていてそこまで詳しく説明しなくても伝わるチームもあれば、レビューリクエストではなくIssueの方に情報を集約しているチーム、そもそもモブプロでやっていてレビューリクエストを書くことに意味がないチーム、など様々だと思います。最終的には、自分たちのチームがやりやすい方法・ルールにしていくのが一番です。

まとめ

この記事では、私が考える「読みやすいレビューリクエストを書くためのヒント」をまとめてみました。何かしら誰かの参考になればうれしい限りです。また、同じレビューアーでも自分のおかれる立場やプロジェクトの状況、チームの習熟度などでやり方や意見は変わってくると思いますので、そういったフィードバックも歓迎です。

では、明日は @EichiSanden の記事をお楽しみに

  1. 日本語表記が「レビューアー」なのか「レビュアー」なのか分からなかったので、この文書では「レビューアー」で統一します ↩

  2. macOS Catalina ではセキュリティが強化されたため、インストールするだけでは使えません。 justinfrankel/licecap Issue #68 を参照してください ↩