はじめに
これまでレビューをする側(レビュアー)としてやってきた工夫を整理します。
ゴチャゴチャになりそうなので、レビューを受ける側(レビューイ)としての工夫は別記事として書きました。良ければそちらもご参照ください。
プログラム等のレビューに限らず、資料作成などにも通じるものとして書いているつもりですが、半ば無意識にプログラミングを前提にしている項目もあるかも。
レビューする側として
コメントにラベルを付ける
コメントの頭に must
などのラベルを付けることで、温度感やニュアンスを明確に伝えられます。下記の記事を参考にしました。
使っていたラベルは以下のとおりです。先ほどの記事から引用しています。
ラベル | 意味 | 意図 |
---|---|---|
Q | 質問 (Question) | 質問。相手は回答が必要 |
FYI | 参考まで (For your information) | 参考までに共有。アクションは不要(確認事項を残す場合などに使用) |
NITS | あら捜し (Nitpick) | 重箱の隅をつつく提案。無視しても良い |
NR | お手すきで (No rush) | 今やらなくて良いが、将来的には解決したい提案。タスク化や修正を検討 |
IMO | 提案 (In my opinion) | 個人的な見解や、軽微な提案。タスク化や修正を検討 |
MUST | 必須 (Must) | これを直さないと承認できない。修正を検討 |
ラベルのメリット
ラベルの目的は、最初に少し触れた通り、コメントの温度感やニュアンスを伝えることにあります。
これがないと、コメントを受け取った側(レビューイ)はその内容が(レビュアーにとって)どれくらい重要なのかが分からず、すべてを対応必須と受け取ってしまう可能性があります。
またレビュアー側としても、「そこまで重要ではないけれど、一応伝えておきたい」という程度の軽いコメントをしたいだけなのに、それが重大な指摘だと受け取られてしまうのは本意ではありません。
コメントにラベルを付けることで、こういったすれ違いを防ぎ、スムーズにやりとりできるようになります。
ラベルの注意点
(この項目はレビューを受ける側も気をつけるべき内容を含みます)
ラベルはあくまでレビューしている側の温度感や認識です。当たり前ですが、それが絶対的な正解なんてことはありません。
レビューを受ける側、つまりレビューイにも、当然ながら実装者としての意見や見解があるはずです。
仮にレビュアーがmust
でコメントしたものでも、内容に納得できなければ、反論が返ってくることもあるでしょう。
レビュアーとレビューイの立場は対等であり、コメントもラベルも命令ではないので、あくまで意見交換に付与されている温度感でしかないという前提は、お互いに理解しておく必要があります。
設計をひっくり返すようなことを言わない
レビューの場面で、「この設計ではなく、こういう風に変えた方がいいのでは?」のような指摘をしたくなることがあります。もちろん、それが重要なら提案していいのですが、忘れてはいけないのはレビューの構造を崩すような変更提案は避けるべきという点です。
どういうことかと言うと、例えば、レビュアーの提案通りに実装者がコードを大幅に書き直した場合、そのコードは事実上レビュアーが設計したものになります。そしてそれがそのままマージされれば、レビュアーの設計が第三者の目を通らないままリポジトリに入ってしまいます。
こうした場合は、設計変更の議論を別の場(チケット、ドキュメント、別PRなど)に切り出すか、あるいは追加で他のレビュアーに加わってもらうなど、「ちゃんと(別の人に)レビューされた設計にする」ことが必要です。
数年後、前提知識を忘れた状態でも理解できる変更かを考える
レビューする時点では、その不具合修正や機能追加の背景をよく知っており、それが前提知識として頭の中にあるので、コードの変更意図を理解しやすい。
数年経って、そういった前提をすっかり忘れた状態で改めて見ても、変更後のコードは理解できそうか?(保守できそうか?)という観点が必要。
この観点がないと、「前提を知らないと読めないコード」がリポジトリに入ってくることになります。
好き嫌いで指摘しない
自分の好き嫌いの域を出ない指摘、つまり客観的なメリットやデメリットを言えない指摘はあまり望ましくありません。
さっきのラベルで言うとnits
(あら探し)などの優先度低めなラベルをつけて「個人的にはこういう書き方が好みですが、このままでもOKです」みたいなコメントをする(主導権は相手に握らせる)程度ならOK、くらいでしょうか。