16
7

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 1 year has passed since last update.

Pull Requestは書き方が9割

Last updated at Posted at 2023-07-15

はじめに

いきなり個人的な話になりますが、
つい最近開発メンバーが増えてレビューしないといけないPRが爆増しました。

レビューしてもレビューしても減らないPR。

「あれ、これ見たっけ。」
「これ修正依頼してまだ対応してくれているやつだ。」
「あ、これレビュー漏れてた。」

などなど、自分の開発タスク&PMOとして進捗管理などをしているので
レビューが大量にきた時にはパンクしそうになります。

そして何よりPRを見てて...

「えーと、このPRは何がしたいんだ。」
「そもそもこの機能どういう仕様だっけ...」
「どんなバグの対応だったけ?」
「何が原因で、何をして、その結果どう変わったのだろうか。」

などと思うことが多いなと気づきました。

元々PRのテンプレートは設定してたのですが、
以前までの体制に適したものにしており、半年以上同じテンプレートのまま放置していました。

新規メンバーが増えたことによりPRの内容・粒度
メンバー間によってバラつきが出て色々課題が見えてきたので、
もっと効率的にレビューができるようテンプレートを見直すことにしました。

そこで調査・検討した内容を残そうと思います。

大前提

PRは「コードを出すためのついでに書くもの」ではありません。
仕様書や開発資料などといったものと同じ、ドキュメントの一種です。

なぜならPRは

  • レビュワーなどの第三者が読む
  • 何か問題が発生した時など、後から見返す場合がある

ものだからです。

そのため、
いつどんな人が見ても内容がスッと理解できるように書く必要があります。

それでは本題です。

PRを作成する前に

いきなりですが、PR作成する前に何か意識されていること、
習慣的にやっていることはありますか?

「コードは書けたし一通り動いたからとりあえずPR出そう!」

これはやめた方がいいと思います。

確認項目はプロジェクトや開発内容によって変わるかもしれませんが、
以下のように基本的にいくつかチェックしておくことがあります。

PR作成前のチェック項目

  • ブランチの最新化
    • 当たり前ですがコンフリクトは解消しておきましょう
  • コンソールやログにerrorやwarnは出ていないか
    • warnでも放置しておくのは良くないですね
  • LintやRubocopなどの静的解析ツール、RSpecなどの自動テストは落ちていないか
    • ちゃんとローカルでチェックしてからPRをあげましょう
  • 動作確認
    • 何か少しでも変更を加えたらこれはマストです。例えば、作業終わってからdevelopの内容を取り込んで動かしたらバグっていた。なんてこともあります。

いざ、PRを作成する

上記のような下準備が完了してはじめてPR作成に取り掛かります。

ここはチームの開発ルールがあればそれに従ってください。

もしルールない・またはルールはあるが十分でないと思う方がいらっしゃれば、
ぜひ参考にしていただければと思います。

DraftでPRを作成する

これは必須ではありませんが、
いきなりOpenにするのではなくDraftという形でPRを作ることができます。

これのメリットは

  • 誤ってマージされることを防げる
  • 他の人に実装途中のコードを共有できる

などがあると思います。

「実装で詰まったので途中だがみてほしい!」
という場合にとても有効です。

差分を確認する

次に、不要な変更が紛れていないかtypoがないかなど
今一度ここでざっと見直します。

よくあるのは、
消し忘れていたコメントやconsole.logが残っているケースです。

PRの概要を記入していく

ここがかなり重要だと思っています。

レビュー効率化のために、
レビュワーをやっていて書いてあると嬉しいことを以下の表にまとめてみました。

見出し 備考
概要 PRでの対応内容の概要
原因と対処法(バグ修正の場合) 何が原因と判断し、どう対処したのか
やったこと 対応内容を簡潔にリストアップ
変更結果 操作動画やスクショ、レスポンス内容など
やらないこと このPRでスコープ外とする内容
注意事項 マージした後はこのコマンドを実行してね、などメンバーに知らせるべき内容
どうやるのか 変更後の使い方や再現(確認)手順
課題 悩んでいるところ、とくにレビューしてほしいところ(これは直接ソースにコメントしてもいいと思います。)
備考 その他追記事項、関連資料や参考資料などをまとめる

自分は以上の内容に加えて「特に見てほしい箇所」や
もう少し上手く書けそうだけど良い方法が思い浮かばない
といった箇所に直接コメントを入れています。

また、「ソース内のコメントに書くほどではないけど、初見だと突っ込まれそうだなぁ
と思った箇所にもコメントを入れています。

あとは動画について。

軽いからという理由でgifを貼っているPRを見たのですが、
数十秒以上の長い動画になると飛ばして見れなく
全て再生されるのを待つのが苦痛なので避けてほしいなと思いました。
(数秒の短い動画なら別に良いと思います)

PRを作成し終わった後

もしDraft状態であればOpenにして、レビュー依頼をしましょう。

ここでは、以下のことをチェックするといいと思います。

  • レビュワーの設定はできていますか?
  • CIは通っていますか?(設定している場合)
  • コンフリクトは起きていませんか?
  • PRの向き先は正しいですか?
    • デフォルトで開発用ブランチに設定していることがほとんどだと思いますが、念の為確認しましょう。

最後に

PRの書き方次第で、レビュワーの時間は何倍も変わってくると思います。

実際にやってみてこそ感じる
ああしてほしいなぁ、こうしてほしいなぁ
という感情は、今後も忘れずにどんどん改善していければと思います。

他にもこういう良い方法あるよ!などあれば
是非コメントを頂けますと幸いです。

それでは最後までお読みいただきありがとうございました!

おまけ

Slackを使っているのであれば、リアルタイム通知を設定することをお勧めします。
レビュワーに設定されたり、コメントがついたりしたら通知を飛ばすことができます。

また、レビュワーに設定されていてレビュー完了していないPRについては
リマインドが来るようにも設定できます。

参考:Scheduled remindersでPull Requestのレビュー依頼をSlackに通知してみた

参考記事

16
7
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
16
7

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?