📔

レビューのためのプルリクエストのベストプラクティス

2022/12/05に公開

GitHub

tech

マイベストのプロダクト開発部のマサです。
最近はスプラトゥーンのサーモンランが楽しすぎてウデマエが上がらないのが悩みです。

この記事では、普段私がプルリクエストを作成する上で心がけている事についてまとめていきます。

はじめに

弊社ではGitHubを使っているのですが、毎日多くのプルリクエストが上がり日々レビューが行われています。
多くのプルリクエストが上がることで以下のような問題は発生することはないでしょうか？

レビューに時間が取られ過ぎる
レビュワーが足りない

これらの問題はレビューイとレビュワーが協力することである程度解決できる課題であると思っています。
この記事で解決策を色々と書いていきたいと思いますが、中には採用できない方法もあるかと思うので、
ベストプラクティス形式で書いて、これは良いなと思ったら参考にしていただければと思います。

レビューイ向け(プルリクエスト作成者)

プルリクエストの目的を記載する

目的: 何をするためのプルリクエストなのかをレビュワーが把握しやすくする

レビュワーはプルリクエストを見る時にその内容について詳しいとは限りません。
簡潔にプルリクエスト内容を把握できるようにすることで、レビュワーが内容を把握するために行う作業量を減らすことができると思います。

以下みたいなことを書くといいかと。

このプルリクエストで何をしたのか
なぜこの変更が必要なのか

プルリクエストを分ける

目的: プルリクエストの巨大化を避けることができ、レビュワーが内容を把握しやすくする

大きなプルリクエストほどレビューに時間が掛かるかと思います。
意味がある単位で分けることで、1つ1つのプルリクエストを把握しやすくなり、
1つの大きなプルリクエストの時よりも全体を把握するのが楽になり、レビュー負担が減るかと思います。

また分割することで、それぞれのプルリクエストに別のレビュワーを付けることが可能になります。
例えばDBに詳しい人にスキーマ設計に関するプルリクエストをレビューしてもらうなど、
別の人をアサインでき、レビュワー人数が増えるということは、それだけレビュー時間も負担も減らせるということになります。

一方でデメリットもあり、
分割してプルリクエストを作には時間と労力が掛かるという問題があります。

しかしデメリットよりもメリットの方が大きいと感じますし、
実際に大きなプルリクエストを見た瞬間は気が重く感じ負担の大きさを感じるので、
できるなら分ける方がベターだと思っています。

スクリーンショットを付ける

UIの変更や、作業手順などをGIFアニメや動画で添付することで、
より把握のしやすいプルリクエストになるかと思います。

GitHubにはプルリクエストのテキスト欄に画像をドラッグ＆ドロップするだけで、
簡単にプルリクエストに画像を添付することができるようになっていて便利です。

しかし、この画像は実はパブリックとして世界中に公開されている画像になっています。
たとえプライベートリポジトリであっても見れてしまいます(2022/12/1時点)

これはGitHubのドキュメントにも記載されています。

したがって、スクリーンショットを付けることは素晴らしいことですが、
GitHubの機能を使う際には注意が必要です。
機密情報や個人情報など万が一漏れてしまっては困る情報は載せないようにしましょう！

これは他の画像共有サービスを使う場合も同様なので、
不明な場合は会社に確認を取り判断を仰ぐことが大事かと思います。

ただ画像を載っけれないは不便なので、GitHubが使えないとしても、
どこかしらにアップロードできる仕組みを用意するのは開発時に約に立ちそうです。

コードに説明を記載する

目的: コードの意図をレビュワーに伝えるため

GitHubだとプルリクエストのFiles changedタブからコードに対してコメントを記載できるようになっています。
ここからコードについての補足説明を書くと、コードからだと読み取れない意図などを、レビュワーが把握するのに役に立つかと思います。

コードにコメント書けばいいのでは？という疑問も出てくるかと思いますが、使い分けが重要かと思っています。
このコメント欄には以下みたいなことを書くと良さそうです

コードについての感想
どうしてこのコードの書き方にしたのか
URLや本など参考にした資料
タスクのチケットの仕様が書いてある場所へのリンク

ここにコメントとして書いておくことで、レビュワーは質問しなくても済むので、
やりとりが減りお互いの負担を減らしレビューをスムーズに行えるようになるかと思います。

またコードにどのようなコメントを書けばいいのかについては、ここに書くと長くなりそうでなので割愛しますが、
個人的には、どうしても書かないと後で困りそうと感じる時だけ書くようにしています。

どういうコメントが適切なのかは難しい問題でありますが、
開発チームで取り決めみたいなのがあると、コメントスタイルがバラバラにならずに、
コードリーディングの妨げになるようなことも無くなるのかなと思います。

この辺は本やブログなどでも紹介されているので、気になる方はチェックしてみてください！

コードオーナーを使う

目的：誰にレビューをお願いすれば良いのかを自動化しレビュワーのアサインをスムーズにする

これ作ったけど誰にレビューしてもらえばいいのだろうか？
特に入社したてのタイミングだと感じることではないでしょうか。

GitHubにはコードオーナーという機能があり、対象のファイルにたいてい個人やチームが責任を負うための仕組みが用意されています。

ここに指定したファイルはその責任者がApproveしない限りマージできないようにすることができ、
これにより、コードの安全性をある程度確保することや、最適なレビュワーをアサインすることができるようになります。

レビュワーに直接説明する

目的：GitHub上でのやり取りを減らすことでレビュワーのプルリクエストへの理解する時間を減らす

特にBugFixなどで早く見て欲しいなど、レビュー完了までを早めたい場合に有効な手段です。
本番でエラーが起こっているのに、GitHubでの文字でのやり取りはすごく時間や労力が掛かってしまうので、
さくっとSlackのハドルやGoogleMeetなどの音声上やりとりなどで済ますと、
問題解決までの時間や負担が減り目的をスムーズに完了させることができるようになります。

また、文章やイラストだけで説明しずらいことがある場合にも有効です。
画面共有で実際のコードやUIを見せながら説明することで、理解も深まります。

いつまでに見て欲しいか伝える

目的: レビュワーのレビュー作業に対するスケジューリングをしやすくする

リリース日から逆算して、このくらいまでにレビューして欲しいのがあったら伝えると良いです。
レビュワーにもやらなければいけないタスクがあり、急に依頼されても対応ができない場合があるためです。

とはいえ、1人のレビュワーが多くのプルリクエストをレビュー依頼されている場合もあり、
プルリクエストごとにスケジューリングを管理するのが難しくなってくる可能性もあります。

そこで、GitHubのラベルを使ってスケジュールの温度感を伝えるのが良いかと思います。
例えば以下のようなラベルを作ったりします。

hurry: 1日以内に見てほしい
days: 2,3日以内に見てほしい
weeks: 1週間以内
anytime: お手すきで

もしくはプルリクのタイトルに書いておくのもわかりやすいかもです。

動作確認した項目を書く

目的: 何を確認したのかを伝えることで、抜け漏れや同じ箇所の確認作業を無くす

レビュワーも動作確認を行う場合があるのですが、どういう項目を動作確認したのか、
どういう手順でそれを行ったのかを記載しておくことで、動作確認をスムーズに行うことができるようになります。

また、基本的な動作確認はレビューイ側に一任して、レビュワー側では、その他に気になった箇所だけレビューを行うことで、レビュー速度を上げることができます。
しかし、ダブルチェックの方が信頼性を上げることができるので、このあたりをどうするかは開発チーム内で相談するのが良いかと思います。

Draftを使う

目的: 作業中だけど相談したいことがある場合に直接コードを見てもらい、作業進捗を上げる

プルリクエストで作業中に実装について迷ったり、複雑なロジックなのでレビューに時間がかかりそうだなと感じる場合は、
あらかじめDraftでプルリクエストを作るといいかと思います。

開発の早い段階でフィードバックを貰うのは、その後の実装がスムーズになるだけでなく、
レビューもスムーズに行えるようになるので、簡単に実装方針を確認するのだけでもプルリクエストとして作っておくことは有効な手段です。

詳しい使い方はGitHubのドキュメントに記載されているのでご確認ください。

レビュワー向け

プルリクエストをじっくり読む

目的: プルリクエストの内容を把握して、レビューに対する精度を上げる

レビュワーはまずこのプルリクエストが何をしているものなのか、何の目的があってこれをするのかを把握することが大事だと思います。
内容を把握していないプルリクエストを読んでもレビュー精度が上がらないですし、
自分自身がロジックを把握するチャンスを逃していることになります。

ここで頑張って読むことで実装者の代わりに、変更や修正ができるようになり、
人による単一障害点を無くしていくことも開発チームにとっては有用なことだと思います。

コードや実装について褒める

目的: レビュワーの成長につながる

「良いコードだな」とか、苦労して実行したものに「がんばったね」とか一言でもいいので、伝えることが大事だなと感じています。
特にレビュワーは開発経験がある人が行うことが多いので、そういう人に褒められるのは単純にレビューイにとっては嬉しい事だと思います。

そんなことで成長につながるのかという疑問もあるかと思いますが、
自動車教習所ではそのような取り組みで結果が出ているというケースもあります。
詳しくは以下の記事を御覧ください。

なぜ修正が必要かを明確にする

目的: レビュワーはなぜそれをする必要があるのか把握してないケースがあるので

レビュワーはなんとなく、これは良くない設計とかコードとかは理解できているので、
コメントでよくあるのは、「ここはわかりにくいので直した方が良さそうですね」とかですね。
しかしレビューイ側はこれで分かるのだろうかと考えるのが重要になってきます。

わかりにくいというには理由があるので、それを説明することでより理解が深まりますし、
AとBがあったら、良し悪しを伝えるのがレビューイの成長につながります。

レビューイの成長は将来的にレビュワーを増やすことにつながるので、
自身が行っているレビューのタスクの工数を将来的に別の作業に割り振れるという意味では、
わかりやすいレビューというのは大きな意味があるのかなと思っています。

レビューイとの関係は対等であると意識する

目的: 上下関係が発生しないようにする

レビュワーはレビューイより開発経験がある人が行うことが多いので、
レビュワーが指摘したコメントは必ず修正とか変更が必要だと思われてしまいます。

レビュワーも人間なので、たまには間違ってしまうこともあるかもしれません。
そういった場合に、間違ったことをそのまま実装してしまい困ったことになってしまうケースも可能性としては考えられます。

また上から目線でのコメントはレビュワーとの関係性を悪化させ、
良いものを作るという目的から外れてしまうことになってしまいます。

開発チームの一員として、良いプロダクトを作る仲間をサポートするという気持ちを持ちレビューすることで、上から目線になってしまうことを避けましょう。
レビュワーが質問・意見を言いやすい環境を作ってあげるのも開発経験があるエンジニアの役目だと思うので、シニアなエンジニアこそ気をつけるようにしたいですね。

言葉使いに気をつける

目的: レビュワーの萎縮しないようにする

レビューコメントは間違いの指摘が多くなってしまうので、そんな気持ちは無いのにコメントがきつくなってしまう場合があります。
特に文字だけでは感情を伝えるのは難しいことなので、例えば「直しましょう」とコメントされている場合にレビュワーは人によっては、もしかして怒ってるのかな？と不安になってしまうかもしれません。

これを例えば、「良さそうですね！でもこっちの書き方にした方がもっと良くなりそうです！」みたいな書き方にすると、レビュワーも「なるほど！そういう書き方もあったか！それは直したほうが良さそうだな」みたいに思ってくれるのではないでしょうか。

絵文字を使うのもの感情を伝える手段なので、積極的に使っていきましょう！

コードを直接書く

目的: 修正内容の相違を無くす

レビューで修正を伝えて、その後再度レビューを行う事になった際に、
自分が思っていた修正内容と上がってきた修正が違っていたことはないでしょうか？

そういう際にコードを直接書くことで、意図が伝わりやすいですし、
お互いのやり取りの回数も少なくなりますので、スムーズにレビューを完了させることができるようになるかと思います。

レビュワーも修正が簡単になるので、時間が無い際はとても便利かと思います。
ですが、レビューイに自身で考える機会はなくなってしまうので、
レビュワーは使うべきかどうかは都度考えて、最適なタイミングで使っていくのが良いかと思います。

レビューイに直接聞く

目的: プルリクエストの内容を早く正確に理解する

レビュワーのとこでも書きましたが、文字でのやり取りより、直接聞いたほうが早く理解できるケースもあります。
プルリクエストをしっかり理解することが重要ですが、
理解するのに自身がない場合は通話などを使用してさっと聞いてしまうのが時間の短縮になるかと思います。

自身の作業時間を確保するためにも、レビューを正確に行うようにするにも、
ショートカットの手段を用意するのは良いことです。

レビュー時間をカレンダーにスケジュール登録する

目的: レビューに使う時間を確保する

レビュー時間を確保するためにカレンダーなどスケジュールを管理しているツールに、
レビュー時間をあらかじめ定期的に登録しておきます。

自分ももっている作業が多くあるので、どうしてもレビューに使用する時間は少なくなりがちです。
やはり自分の作業にしっかり時間を使い、さっさと終わらせてしまいたいと考えるのはふつうの事かなと思います。

しかしレビューを終わらせないと他人の作業は終わりませんし、
もしかしたらそれによってリリースされるものは自分に作業に影響がある場合もあります。

レビューを早く終わらせることは単純にリリースするものが増えるので、
プロダクトの成長につながるので、レビューをしっかりとできるように時間を確保することで、
忙しくてレビューする暇もないということがなくなり、チーム全体の開発がスムーズになります。

開発チームによってはレビューアワーなど1週間のうちでレビューしかしない箇所を設けて、
しっかりと開発が進むように取り組んでいるところもあります。

開発におけるレビューの重要性をしっかり認識して作業に取り組むようにしましょう。

さいごに

レビューのためのプルリクエスト・ベストプラクティスということでいくつか書かせていただきましたが、
あなたにとって使えるものも、使えないものもあったのではないでしょうか？

大事なのはレビュワー・レビューイでお互いが協力してスムーズにレビューをできるようにしていくことなのかと思います。
そのためにはお互いのことを考えるのと、どうしたらもっとレビューがやりやすくなるのかを、
日々考え、工夫していくことなのかなと思います。

開発が活発になるとレビューに関する問題が増えてくるかと思いますが、その解決の人助けになれば幸いです。
また他にもアイデアがあればぜひ教えていただけると大変助かります！