Pull Request(PR)の作成時に気をつけるポイント

review-ok Engineer

こんにちは。
泡アハです。

PullRequestって作るのは簡単だけど、
コメントや概要には何書いたらいいかわからないと悩んでいる方をよく見かけます。

私は、Sier時代、9人ほどのチームリーダーをやった経験があるのですが、
レビュワーからみるとこういうPRは、みやすいといった基準があるように思います。

そこで、今回は、レビュワー視点も経験した私が見やすいPRを作るときに抑えておきたいポイントについてお話ししたいと思います。

この記事を読めば、わかりやすいPRがかけるようになって、
チーム全体の開発効率も向上するかと思います!

結論

Pull Request(PR)とは、自分が書いた変更を他の開発者に通知する機能のこと。

PR作成時に意識することは、
PR内の情報だけで、変更が問題ないかを判断できるように情報を提供する。
の1つだけ。

上記を実現するための具体的なポイントは、以下3つ

  1. なぜこのPRが必要なのか?を概要に書く
  2. 自分が実装しているときに困って調べたことはないか?をPRコメントに書く
  3. はたからみて、なぜやったのかわかりにくいところはないか?をPRコメントに書く

Pull Requestとは?

そもそもPull Requestとは何なのでしょうか?
このサイトがとてもわかりやすかったので、引用します。

簡単に言うと、開発者のローカルリポジトリでの変更を他の開発者に通知する機能
出典: https://backlog.com/ja/git-tutorial/pull-request/01/

つまり、こういう変更をしたので、他の方に見てほしい!
という機能なんですね。

では、PRを作る目的は何になるのでしょうか?
ここは、PRがなかった場合を考えるとわかりやすいです。


出典: https://backlog.com/ja/git-tutorial/pull-request/01/

PRがない世界では、開発者以外の誰の目も通らず、リポジトリが変更されてしまい、
第三者がレビューすれば、防げたバグも防げなくなってしまいます。

一方、PRが存在する世界では、
レビューをしなければ、リポジトリに反映することができないため、
開発者以外の第三者のレビューを強制し、バグを起こしづらくなるという感じですね。
また、PRを残すことで、その習性がどんな理由で行われたのか、いつ、だれが行ったのか、レビューワが誰なのかといった情報がPRという形で残るため、後世の人が経緯を追いやすくなるという利点もあるように思います。

ここまでをまとめてみます。

PR(Pull Request)とは?
実装した機能をほかの開発者に通知する機能のこと

PRを作る目的は?
- 変更点を第三者にレビューしてもらうこと。
  - 追加した機能が問題ないか。
  - 考慮漏れがないか。
  - 自分の思考プロセスが間違っていないか。
→ 開発者だけでは、気づかなかったバグに気づくことができる
- なぜこのPRを作ったのかを後世に残すことができる。

では、ここまでを踏まえて気にするべきところは、何があるでしょうか?
先に結論を書いてしまいます。

PR内の情報だけで、変更が問題ないかを判断できるように情報を提供する。

私が思うにレビューを依頼する人が気にするべきことは、これだけです。

とはいえ、これだけだと抽象的すぎて何をしたらいいかわからないと思うので、
具体的に気にするべきポイントをそれぞれ説明していきたいと思います。

なぜこのPRが必要なのか?を概要に書く

PRを作るということは、
・バグの修正
・新機能の開発
というように必ず解決したいことがあるはずです。
(新機能を開発するということは、なんらかの困りごとがあって、機能開発するはず。)

そのため、PRには、
なぜこの修正が必要なのか、何を解決したいのかを書くことがとても重要だと考えています。

なぜこの修正が必要なのかがわかれば、
レビュアーは、このアプローチがあっているのか判断できるし、
どこに考慮するべきなのかわかるからです。

GithubのPullRequest機能であれば、
概要を書く欄があると思うので、
その欄になぜこのPRを作ったのか、どんなことを解決するPRなのかを第三者がみてわかるように書くことが大切だと思います。

自分が実装しているときに困って調べたことはないか?をPRコメントに書く

自分が実装していて、困って調べた経験はエンジニアであれば、ほぼ毎日直面しているのではないかと推測します。
自分が困ったということは、ほかの方も困る可能性が高いです。

困ったにもいくつか種類がありますが、
主に以下のような事柄については、PRコメントとして書くべきだと思います。

①ドメイン知識(業務知識)が必要で、Slackや誰かに聞くなどして、解決したこと
②プログラミング言語的に書き方で困って、Webサイトなどをみて、解決したこと

①は、業務的に複雑で、
わかりづらいことについて補足として書きます。
必ずしも言葉で書く必要はなく、
SlackのURLだけはるでも問題ないとも思います。

②は、プログラミング言語として、普通こういう書き方をすると思うけど、
あえて、こう書いてますみたいなケースの時に書きます。

また、バグ修正においては、
もしCIが導入されているPJであれば、
あえて動かないパターンでプッシュして、
テストが落ちることを確認して、
修正版でテストが通ることを証明することもありだと思います。

はたからみて、なぜやったのかわかりにくいところはないか?をPRコメントに書く

みなさんは、他人がコードを見て、「ん?なんでこう書いたの?」と思う経験ありませんか?

たとえば、rubyにおいては、同じクラスの別メソッド配列の要素に適用したい場合、&methodを利用することで簡潔に書くことができます。

# イメージ
[1,2,3].each(&method(:puts))

もしレビュアーがruby経験が浅い場合、ん?なにこれ?と思うかもしれません。
そうした場合、この構文を見た時、レビュアーはググるという行為が発生してしまいます。
そこで、レビューを依頼するときは、参考文献をはったり、何がしたいのかをコメントに書くことで、
レビュアーの負担を一つ減らすことができます。

また、このほかにも本来PRとしては関係ないけど、
気になったので直したというケースも考えられます。
(lintなどもこのケースですね。)

この場合、レビュアーは、
ぱっと見関係なさそうだけど、なんで直した?と疑問に思い、質問してくるかもしれません。
こういった場合でも、こういう理由で気になったので、直しました。
と書いてあれば、レビュアーの負担を一つ減らすことができますね。

まとめ

いかがでしたでしょうか?

PullRequestを出すときに、
意識するべきことをまとめてみましたが、
思っているよりも長尺になってしまいました。

レビュアーの負担をいかに減らすかが、
チームの生産効率性に影響を少なからず与えると思うので、
レビューをお願いするときは上記に書いた観点を元に
PRを作成するとよいかもしれません。

おわりっ!

コメント

タイトルとURLをコピーしました