概要
3年くらいGitHubのPullRequestでコードレビューをしている経験があるため、いつも個人的に気にしていることをまとめようと思った。
(技術的な面は少なめで、主にマナーだったり気持ち的な面が多いです)
レビュアー、レビューイの両方の立場で気をつけていることをまとめてみた。
レビュアーとして気をつけていること
感謝の気持ちを持つ(できれば伝える)
レビュアーからすると、できあがったものを確認するだけになることが多いので忘れがちだが、PRはレビューイが時間をとって対応してくれたものである。
また、レビューイが取り組んでくれたPRは、もしかしたら自分が対応しないといけなかった可能性は少なからずあるので、代わりに対応してくれてありがとう、という気持ちを持つようにしている。
ただ、それだけだと自分の心の中だけになってしまうので、「対応ありがとうございました！LGTM！」みたいな感じで、Approve時のコメントで伝えたりするようにしている。
コメントするときは冷たい表現にしない
よく言われることだし、相手との関係性とか、効率面とか、人によって考え方の幅が大きく違いそうなところではあるが、やっぱりテキストベースだとどうしても冷たい表現になりがちなので、レビューイからするとちょっと攻撃的というか怒られているように思えてしまう場面がある。
例えば、前後の文脈にもよるが、「〜に変更してください！」とかとコメントするより、「〜に変更してもらえると🙏」などのように柔らかい表現、プラス、文脈によっては絵文字を使うと、少なくとも僕は傷つかないのでいいと思っているw
意図が読み取れない部分はそのままにせず質問する
よくわからないまま Approve しても、理解することを諦めた(妥協する)ことになり、自分のためにも相手のためにもならないので。
それに、意図が読み取れないということは、よくない実装の可能性もあるので。
質問するほどではないけれど気になったところはコメントする
上記のように意図が読み取れない、とまではいかないが、なんか気になったり、理解に時間がかかってしまった部分は一応コメントを残しておく。
自分が「こう理解していますよ」というような証明にもなるし、今後誰かがgit blameでコミットを遡ってそのPRに辿り着いたときに、「なるほど、そういうことか」となったりすることもあるので。
(それなら実装を修正してもらったり、コードにコメントを残してもらうべきということもあるが、、、)
指摘するときは意図も伝える
「ここは◯◯を使ってください」というコメントだけだと、ハイコンテクストなコメントになりかねず、レビューイからすると「なぜ？」となることもある。
その意図を把握するのにコメントが往復してしまったり、レビューイが頭をフル回転させ、その意図を推測しないといけなかったりもする。
なので、ちゃんと指摘の意図も付け足して、レビューイが感じる「なぜ？」を極力減らす。
なるべくサンプルコードを添える
上記の＋αで、「◯◯を使ったほうが、〜の理由でよさそうです！」みたいな指摘コメントだけよりかは、「例えばこのような書き方です！」みたいに サンプルコードも一緒に書いてあげると、レビューイとしても「◯◯を使う？どういう書き方をすればいいんだ？こういうこと？」みたいなことになりにくいので。
なる早で確認する
自分の現在のタスク優先度や、そもそも急ぎではないPRだったら、、、とかにもよるが、基本的には依頼が来たその日中か翌営業日中には確認する。
ちょっと時間が経ってしまうと、レビュアーが忘れているのかな？とか、何かよくない実装があってそれの確認に時間がかかっているのかな？、とかと思って、レビューイがソワソワしてしまうため。
もし時間がかかりそうなら、理由をつけてその旨を連絡したり、「一旦ここまでは確認しました！残りはこれから確認します！」など、途中経過を伝えるのでもよいと思う。
レビューイとして気をつけていること
レビューしてもらったらお礼を言う
たまに、Approveをもらっても、何のコメントもなく、そのままマージする人がいる。
基本的には、「ありがとうございます」とコメントしたほうが個人的にはいいなと思う。
逆に、「ありがとうございます」だけの通知がうっとおしいと思う人もいるので、なんとも言えない部分ではあるが。
せめて「LGTM！」とコメントされていたりしたら、そこに絵文字マークをつけるだけでもしたほうがいいように思う。
GitHub上でもセルフレビューは必ずする
コミット時にdiffを確認していた場合でも、実際にGitHubでPRを作ってFiles changedを見ると、「何だこれ」と思ったり、「ここわかりにくいなあ」と自分で気づけることが往々にしてある。(少なくとも僕はある)
なので、必ずセルフレビューをして、逆に自分がレビュアーならここツッコむかもと思うところをなくしてからレビュー依頼したほうが、レビュアーにとって「これは何だ？」と思う場面がその分減るはず。
自分の能力不足等により妥協してしまった部分はコメントで意思表示する
事前に実装の相談とかできれば別だが、何かしらの都合でどうも納得いかない実装をしてしまったときに、その点に何も触れないままレビュー依頼しても、高確率でどうせ指摘が入るので。
あとは、コードベースで実装の相談ができるので。
自分でもわかりにくいと思った部分はコメントで説明する
上記と似ているが、自分がわかりにくいと思っているということは、レビュアーはもっとわかりにくいと思っているはずなので。
参考資料は該当箇所にコメントする
とある資料に基づいて実装したり、別の箇所で参考にしたコードがある場合などは、今回修正したコードの該当箇所にリンクを貼ったり、引用をつけるなどして、レビュアーに資料を探させなくて済むようにする。
できるだけ、スクショや動画を貼って視認性をあげる
画面の追加やデザイン変更などは、コードだけ見てもちゃんと実装できているのか、いまいちわからないので。
もちろん時間があれば、レビュアーがローカルとか実際に動かせる環境でも確認してくれることもあるが、スクショとかを適切に貼っていると、レビュアーの動作確認時の負担も減ると思うので。
適切な粒度になるようにPRを分割する
実装機能のボリュームとかにもよるが、レビュアーの(主に脳の)負担にならないように、できるだけPRを小さくする。
逆に、全体像が見えにくくもなるので、どの粒度にすべきかは個人的にはけっこう迷うところではあるが。。
さいごに
もちろん上記以外にも、状況や実装内容によっていろいろと気にしていることはありますが、ざっと思い当たるところだとこんな感じでした。
僕自身もですが、ついつい疎かにしがちな部分なのと、若干理想論的な部分も含みますが、できるだけレビュアー/レビューイがお互いにストレス少なくレビューに臨めるといいなと思っています。
