あけましておめでとうございます ….

と、この下書きの先頭に書いてありましたので、この記事を書き始めてから、こちらもすでに9ヶ月以上が経過しております。今年ももう後半戦入っておりますが、ベリースローペースでがんばらせていただきます。

そうこうしているうちに、株式会社クロコスの、ヤフーへの吸収合併が発表されました。 この媒体で記事を出すことは、もうあと何回もないかとは思いますが、皆様これからも変わらず、よろしくお願いします。

さて、そういうわけで。

長くなるので先に結論

• Pull Request でレビューするとき、レビューを提出する文書フォーマットを決めておくと非常に捗る
  • レビューが早くでき、
  • レビューの質があがり、
  • ハッピー

Pull Request のレビューについて

本題です。

前提条件:

• 基本的には、OSS 等ではなく、業務で利用する GitHub を想定しています。
  • (もちろん OSS に当てはめても良いとは思いますが、OSS の場合は、より Pull Request を受け取るハードルを下げる方向のほうが、全体的には活発になって良いのかな、と思います。もちろんレビュワーは大変ですが。そこはそのソフトウェアの開発ポリシーによりますかね)
• 開発には常に Pull Request を利用していて、メインブランチへの merge 前に必ず Pull Request によるレビューを行うものとします
• その上で、どのようにレビューをしたら効率が良いか (レビュワーにとって負荷が少なく、漏れが少なく、効果的で、効率的) を考えます

実際に運用しているうちの環境としては、

• 開発チームを、リーダー1人 + メンバー2-4人程度の小規模なチームを保持
• Pull Request のレビューは以下の体制で行っています
  • チームリーダーが行う
  • リーダーのコードは、リーダー同士でチェックする (都度「XXさんこれ見てください」のように依頼してる)
• 最低1人以上から「:+1:」コメントがつくと merge 可能
• 大きなブランチはなるべく作らない。大きくなりそうなときはリリースを分割したり、サブブランチに分割したりする。

また、本稿の背景にある考え方として、以下の記事が非常に参考になりましたので、是非一度読んでみてください。

• コードレビューを円滑に行いたい #cross2014 のお話

Pull Request のフォーマットって？

フォーマットとは、Pull Request を作る際の説明のテンプレート/ルール です。

具体的に見ないと、イメージが湧かないと思うので、うちで使っているテンプレートの1つの例をみてみます。

## 関連URL

* WikiのURL(Markdown形式)や ...
* 画像URL や ...
* 操作可能な画面のURLや ...
* (あれば...) ...


## 概要

* なぜこの変更をするのか、
* 課題は何か、
* これによってどう解決されるのか、
* など、この変更に対する概要を記載


## 影響範囲・ターゲットユーザ

* 主催者へのメリット・デメリット
* 応募者へのメリット・デメリット
* その他のユーザへのメリット・デメリット


## 技術的変更点概要

* なにをどう変更したか
* ロジックがどういう手順で動くのか、
* DBからどういうクエリで何をとってそれに何を処理するのか、
* などなど、
* レビュワーにわかるように、技術的視線での変更概要説明


## 使い方

* 使い方の説明
* 再現条件 (例えば、いいねの状態、デバイスの状態、プランの状態や、設定によってこういう風に使える、とかこういう画面が出てくる、とか条件があれば)


## UIに対する変更

* 変更前のスクリーンショット
* 変更後のスクリーンショット


## マイグレーション

* マイグレーションが必要な場合、`ALTER TABLE` 等を記載する

記入例:

```sql
ALTER TABLE message_send ADD body_replacements ...
```

## クエリに対する変更

* 変更されるクエリ (変更前、変更後)
* 新規に追加されるクエリ
* 観点:
    * WHERE句に指定した条件の index 設計は適切か
    * 想定される対象のデータ量
    * 想定されるデータ量が大量のとき大丈夫か (ダメな場合、いつまでにどうにかする予定があるのか、忘れないために別チケットを用意したか)


## 個人情報の取り扱いに変更のあるリリースか

* メールアドレスや住所情報等、個人情報にあたるものを新規に保存する場合、ドキュメントを書き、そこへのリンクを貼る


## TDログ

重要な機能を追加・変更した場合、ログを仕込む必要があるか、ログが入っているか、入れているログが変更によって漏れていないかなどを確認する必要があります。

* [ ] 重要な機能であり、ログを仕込んだ (説明用Wikiへのリンク)
* [ ] ログを入れる必要ナシ


## テスト結果とテスト項目

* [ ] テストする際の項目を、このように、チェック可能な形式で記載する。
* [ ] テストしたらチェックを入れていく。(staging でテスト、など出ない場合、基本すべてのチェックが終わった時点でレビューに入る)
* [ ] 特に、PC、モバイルの違い、応募フローの再現手順など、できるだけ広い視野で。
* [ ] 詳しくは、テスト観点リストも参照のこと。
* [ ] できれば、**開発した人以外が** 項目を洗い出してください。(開発者本人は、盲目的になっている可能性があります)
* [ ] 機能のON/OFF によって画面の出しわけ、機能が有効になったり無効になったりする場合、場合分けしてテスト項目を書く
* [ ] 他チームへテストを依頼した場合はチーム名書いておこう
* [ ] それをテストしたチームの人が、チェックを入れよう


## 今回保留した項目とTODOリスト

箇条書きで書く。できれば次のチケットを作る。

* あれこれを治す #xxxxxx
* あれそれを実装する #xxxxxx


## 共有先

↓ 共有したい先にチェックをいれてください。
merge 時
`HipChat: 部屋 @sotarok` などを記載すると、共有時に自動的に mention もつけてくれます。
記法: `HipChat: 部屋 @sotarok`

社内共有:

* [ ] HipChat: Crocos ALL
* [ ] HipChat: リリース情報
* [ ] HipChat: 開発
* [ ] HipChat: グロースx開発
* [ ] HipChat: 営業x開発グロース
* [ ] HipChat: サポート
* [ ] メールで共有: (ここにメールアドレスを記載)

社外アナウンス (上から大きい順):

* [ ] プレスリリース
* [ ] 告知用ページ作成 (w/ グロース)
* [ ] 事前メールアナウンス (internal のアナウンス機能で)
* [ ] Facebookページでアナウンス
* [ ] アナウンス無し

## その他

* レビュワーに対する注意点 (ここはこういう風に思ったけどこういう風に実装した、ここはこうしようと思ったんだけどこういう悩みがありやめた、など注意点があれば)
* リリースに対する注意点 (その他)

上記フォーマットは、「機能追加・機能変更系」のための Pull Request のフォーマットです。 これは社内の Wiki に置いてあって、Pull Request を送るときに↑からフォーマットをコピーして中身を埋めて(変更したり削ったりして)、Pull Request を送信する、という流れになっています。

フォーマットの内容にどういうことを書くのか、というのは、フォーマットの定義自体に解説が書いてありますので大丈夫ですかね。

ちなみに、このフォーマット中にある テスト とは、ユニットテストではなく、ファンクショナルな、人力テストということです。 コードレベルでの保証は自動テストで行ったほうが良いでしょう。クロコスの場合は、Pull Request ごとに Jenkins のテスト結果が Pull Request 上に表示されているため、まあこちらは問題がありません。

Pull Request のタイトル

タイトルはフォーマットに記載していないのですが、以下のようにルールづけています。

変更の内容が他の人にも伝わるように1行でまとめる。エンジニア以外が見ても分かる内容にする。

さて、これにそって記入後、ペロっと GitHub に貼れば、もちろん Markdown 形式なのでフォーマットされて表示されます。素敵で便利ですね。

フォーマットの種類

以下の6通りのフォーマットを用意していて、それぞれ多少項目が違ったりします。(例えば、フロントエンドのみの変更では、技術的解説欄はなかったり、不具合修正系には障害報へのリンクや影響範囲の詳細調査結果などの内容があったり、など)

• 機能追加・機能変更系
• リファクタ・構造の変更・基盤開発系
• 不具合修正系
• あるPRの修正PR
• 小さな変更
• フロントエンドのみの変更

フォーマットを定義するメリット

上記フォーマットをみてもらうと分かる通り、結構盛りだくさんですが、このようにフォーマットを決めておくと、

レビュイー (Pull Request を送る方) にとっては:

• レビュワーに 何を見てほしいか が明確になる
• その機能が利用者に何をもたらすかというプロダクト視点を育てる
• Pull Request を堤出する人による観点のズレが減る
  • この変更について、事前に把握しているか/いないか、あるいはコード対象コードを見たことがあるか/ないか、などレビュワーにもそのコードへの精通度合いが違います
  • その違いを吸収し、ある程度、だれでもレビューできるようになります (もちろんコーディングや設計のレベルとしてエンジニアとしての一定のレベルは必要。)
• リリース前後の手順の確認
• 何をテストしたのを明確にする、テストをさぼらない

レビュワーにとっては:

• 何をレビューすれば良いか が明確になる
• この変更によって何が変わるのか、何が行われるのか、その理由もinputできる
• 変更前後の画像が添付されるためひと目でわかる
  • UIのどこが変わったのかが明確になり、そこから影響範囲の想像力を働かせるうえでも役に立つ
• 関連文書 (Wiki などに書かれたドキュメントや、申請系) にすぐアクセスできる
• 何をテストしてくれたのかがわかる
  • 例えば、「ここを変更すると、ここにも影響範囲がありそうだな、ここってテストしたのかな」等、フォーマットが導入される前は、聞いてみて「あ、それもテストしました」というやりとりもありましたが、今では、何をテストしていて、何をテストしていないのかがすぐわかるため、コミュニケーションコストの軽減にもなります

その他のメリット

フォーマットにそって記載されていると、Parsability が上がるということもメリットの一つとしてあげあられます。 クロコスでは、Pull Request のフォーマットにそってパースされ適宜様々なところへの情報共有に利用されます。

Pull Request マージ時に hook して…

• HipChat への通知

  • どの部屋に通知するのかを指定しておくと、merge hook 時に自動的にフォーマットの中から重要なものだけを抜き出してbot が発言してくれます。 例えばこんなかんじ:

  

  • これにより、例えば、営業部から依頼を受けて作業していたものが、「あれの作業って終わりました？」「あ、実は一昨日リリース済みです」のような、共有漏れによるロスがなくなりました。
  • @sotarok などと記載しておけば、mention も飛ばせます
• リリーストピック収集ツール
  • 週1回の全体定例MTGで、今週なにがリリースされたのかを発表する場があるのですが、その際に、注目すべきリリースが何か、後からリリースを振り返らずとも、自動収集でレポートを作ることができます
  • また、ここで発表される、ということを前提におくと、「Pull Request にかかれている内容が当人にしかわからない」という意味不明なことが少なくなります。(ただし、エンジニア以外にも見られる情報だからね、というのを浸透させるのは少し時間が必要でした ^^; 繰り返し言うこと、が重要です。)

レビュイーが、色々書かなければいけないという、時間的/労力的に大変というデメリットもあります。しかし、そもそもフォーマット自体、「いくら急いでいても、多少大変でも、このレベルは保ちましょう」という一定のガイドラインを設けることでもあり、これを満たせないならば (その会社での/そのチームでの) その人の活躍の場は無いよ、というだけの話であり、まあそれはそれで。

したがって、フォーマットのレベル感といいますか、どのレベルまで書かせるか、というのは、当然

• アプリの特性
• サービスのステージ (立ち上げ期？成長期？運用期？)
• チームの特性
• 会社のステージ (立ち上げ期？成長期？運用期？)

などによって異なります。

会社やサービスが成長していくにしたがってフォーマット自体の見直しも適宜行っていかなければいけませんね。

例えば、会社の立ち上げ期、エンジニアが3人くらいだとすると、みんなが同じものを作っています。そうすると、お互いの責任範囲や、書いたコードへの理解、作っているものをみんなが共有し、知っている状況ですよね。そういう時は、前提条件となるものの説明などはある程度省けるかと思います。((それでも変更の概要はもちろんあるほうが効率は良いと思いますけどね))

このフォーマット導入後は、3ヶ月に1度くらい、「いまのフォーマットでの不満、過剰な点、内容としてこれも欲しいよ、という点」などをメンバーにヒアリングしていて、それによって既に3-4回の改定が行われています。ルール自体も、陳腐化しないように育てていく必要があります。

フォーマットをうまく運用するコツ

定義しても誰も使ってくれないのでは意味がありません。 まずは、必ずこのフォーマットにそっていなければレビューされない、というのは大前提としてあります。レビュワーにとって非常にメリットがあるため、Pull Request が堤出された段階でこのフォーマットにそっていなかったら突き返す、というのは自然と発生しますw

ただし、色々書くのが面倒、というのを超えるメリットが無ければ、なかなか大変です。 そこで、上の方に書いた、フォーマットにそっていれば、自動でチャットや定例MTGで共有されるといった負荷軽減ができると、理解してもらいやすくなります。

まとめ

Pull Request を送るときの文書フォーマットを決めて運用することで効率が良くなる話をしました！みなさん、お気づきかもしれませんが、タイトルでは「レビューの効率が…」と書きましたが、実は、レビューだけでなく全体の業務や情報共有の効率がめちゃくちゃ良くなるのですよ。

次回は、Pull Request を用いた、半自動リリースのフローについてお話できると良いなーと思います。