オールアバウト Advent Calendar 2016 の 6日目の記事です！

プルリクテンプレート使ってますか？

みなさん、プルリクテンプレート使ってますか？

規定のプルリクテンプレートがあるプロジェクトや会社もあると思いますが、最近弊社でもプルリクテンプレートの使用をルール化しました。

ルール化から約2ヶ月経ち、結構良い感じに回っているので、今回はそのことについて書きたいと思います。

プルリクテンプレートが無いとどうなるか

前提として、弊社では自社製レビューガチャ(Slack Bot)でランダムに選択された2人をレビュアーに指定しています。
そのレビューガチャの仕様上、全く違うプロジェクトの人も必ず1人当たるようになっています。

それ自体は知識の共有や別プロジェクトの視点を取り入れる、という意味でとても良いことなのですが、当初は各実装者が適当なフォーマットでプルリク説明文を書いていたため、レビュアーになったとき(特に別プロジェクトのプルリク)にこんな問題が起こっていました。

1. 改修目的がわからない

• 何故その改修を行ったのかが書いていないので、改修の方向性や実装アプローチ自体への指摘がしづらい。

2. 何を動作確認したのかわからない

• 実装者が何をテストしたのかが書いていないので、何が担保されているのかがわからない。
• テストパターンが充分かどうか不安なので実装者に聞きたいけど、既にテストしてたら気まずいので黙っておく。。

3. いろんな実装が混ざっていてカオス

• 実装のメイン部分とは関係のないリファクタリングがさり気なく混じっていて混乱。
  • 突然のインデント変更とか。
• どこに注力してレビューすれば良いのかわからない。

・・と言った具合で、結局もやもやしたままレビューしたり、最初から説明文に書いておけば済むような箇所で実装者とレビュアーのやり取りが発生、という状態になってしまっていました。

結果としてレビュー完了までの期間が長くなったり、かかる時間も増えてしまうので、クリエイティブな仕事をする時間が減ってしまいます

プルリクテンプレートの使用をルール化

諸々の問題を解決しレビューを高速化するため、「開発者・デザイナーは共通のプルリクテンプレートを使う」とルール化しました。

実際に使用しているプルリクテンプレートはこんな感じです。

# チケットURL (任意)
+ RedmineのチケットURLを記載。

# 概要 (必須)
+ 実装した理由を含む概要文を1~2行程度で記載。
    - ※あくまで"理由"なので、実装内容はここには書かない。
    - 例：記事ページをスクロールしても右カラムが追従しなくなってしまっているので、追従するように改修。

# 対応 (必須)
+ 概要に対して何を実装したのかを記載。
    - 例1：JSでスクロール位置取得関数を呼び出すタイミングを、画面ロード直後に変更。
    - 例2：スクロール位置計算ロジックが間違っていたので修正。

# その他対応 (任意)
+ この開発以外に行った小さいリファクタリング等があればここに記載。
    - 例1：フォーマッターをかけた。
    - 例2：改行コードが混在していたので直した。

# テスト (必須)
## URL (画面で確認できる場合は必須)
+ レビュアーが開発物を動作検証できるURLを記載(テスト確認用サーバーのURL等)。
    - ※URLパターンが多い場合、別途スプレッドシートにまとめてそのシートのURLを記載、でも可。

## Done (必須)
+ 何をどう動作確認したかを記載。
    - 例1：テストURLにChromeでアクセスし、スクロールで右カラムがきちんと追従してくる。
    - 例2：JSエラーが発生しない。
    - 例3：ユニットテストが通ったことを確認。

## ID / Pass (任意)
+ 動作確認用のURLの表示に認証が必要であれば、ここにアカウント情報を記載。
    - ※テスト用の共通アカウントがあれば尚良い。

# 不安なところ (任意)
+ 実装内容に関して不安なところがあれば記載。
    - 例1：○○メソッドをモデルに定義するかサービスに定義するかで悩んだ。
    - 例2：○○の処理の部分でもっと良い実装方法があったら教えてください。

# 備考 (任意)
+ レビューする上で他にレビュアーに知っておいてほしいことがあれば記載。
    - 例：CSSの調整は今回はやっていない。

# 関連プルリクエストURL (任意)
+ 別リポジトリで同様のプルリクを出していたり、依存関係にあるプルリクがあればURLを記載。

# スケジュール (必須)
+ レビュー締切日時：12/13 12時
+ リリース予定日時：12/15 10時

実装者は最低限必須項目を埋めたものをプルリク説明欄に入力し、プルリクを出します。

ポイントは、「概要」と「対応」を分けていることです。

「概要」の目的は"何故このプルリクが発生したのか"をレビュアーに伝えることなので、具体的な実装内容については書きません。

具体的な実装内容は「対応」の方で列挙します。

また、「スケジュール」の項目にレビュー締切とリリース予定日を書くことにより、レビュアーはいつまでにレビューすべきなのかはっきりわかり、作業スケジュールが立てやすくなります。

プルリクテンプレートを導入した結果どうなったか

導入前に発生していた問題は圧倒的に減りました。

具体的に感じたメリットとしてはこんな感じです。

1. 改修目的が明確なので指摘しやすい

• 改修の方向性に対する指摘もしやすくなった。
• 目的を把握することにより、コードを読むスピードも上がった気がする。

2. 動作確認内容が明確なので考慮漏れを指摘しやすい

• テストパターンの漏れ(=仕様の考慮漏れ)を発見しやすくなった。
  • 品質向上！

3. しっかり見るべき箇所に注目できる

• 実装のメイン部分や、実装者が不安に思っている箇所にフォーカスできるようになった。

4. 全体的にコミュニケーションコストが減った

• レビュアーが知るべき情報が網羅されているため、実装者に質問することなくスムーズにレビューできる。
  • レビュー時間の短縮！(全体で見ると、レビュー数 x 人数分の効果！)

実装者的には、的確なレビューが早く返ってきてHappy
レビュアー的には、レビューの負担が減ってHappy

まとめ

プルリクテンプレートをルール化して本当に良かったです。

もしまだあなたのチームで導入していないなら、一考の価値はあるので是非検討してみてください。

ちなみに弊社ではBitbucketを使っているのですが、BitbucketにはGitHubのようなプルリクテンプレート登録機能がないのでブックマークレットで自動挿入できるようにしています〜

javascript:document.querySelector('#id_description').value = '
# チケットURL (任意)\n
+ \n\n
# 概要 (必須)\n
+ 実装した理由を含む概要文を1~2行程度で記載。\n\n
# 対応 (必須)\n
+ 概要に対して何を実装したのかを記載。\n\n
# その他対応 (任意)\n
+ この開発以外に行った小さいリファクタリング等があればここに記載。\n\n
# テスト (必須)\n
## URL (画面で確認できる場合は必須)\n
+ \n\n
## Done (必須)\n
+ 何をどう動作確認したかを記載。\n\n
## ID / Pass (任意)\n
+ \n\n
# 不安なところ (任意)\n
+ 実装内容に関して不安なところがあれば記載。\n\n
# 備考 (任意)\n
+ レビューする上で他にレビュアーに知っておいてほしいことがあれば記載。\n\n
# 関連プルリクエストURL (任意)\n
+ \n\n
# スケジュール (必須)\n
+ レビュー締切日時：\n
+ リリース予定日時：
';void 0