コードレビューをやるようになってから1年ちょい経ったので、レビュー観点とレビューをするときに意識していることを書く。
歴1年のペーペーが書いている記事なので、参考までに。
観点
レビュー観点は、言語化できないところもあり、ここに書いていることが全てではないが、大体毎回こんなところを観ている。
要件を満たしているか
プルリク飛んできた時は、コードを読む前に要件を満たしているかを確認する。
ブランチ切り替えてローカルで実際に動かしてみる。
要件を満たしていなければコードを読むまでもない。
ここをクリアしてからコードを読み始める。
コーディング規約に沿っているか
コーディング規約をドキュメントに記載しているプロジェクトであれば誰でもチェックできる観点。
レビューで何を見ればいいか分からなければ、まずこれを確認してみるといいと思う。
デザインパターンに沿っているか
コーディング規約と似たような観点だが、そのプロジェクトで採用されているデザインパターンに沿った実装ができているかを確認する。
変数名・関数名
ぱっと見で理解できる命名をしているか。
自分で命名した変数は、少しおかしくても理解できてしまっているので、レビューで拾って欲しいところ。
同じようなコードを書いていないか
同じようなコード、もしくは「今後違う箇所でもつかいそうだなあ〜」と思うようなコードは切り出すこと(共通化)ができないかを考える。
コメントが正しいか
コメントが間違っていてもコードは動くので、ミスしやすいところ。
実装に沿っていないコメントが1つでもあると、全てのコメントを疑わなければならなくなってしまう。
削除できないか
関数を使用しているコードが削除された差分があれば、その関数自体を削除できないのかを確認する。
関数に限らず、cssとかも。
とにかくいらないものは消す！
省略できないか
これは、好みとかプロジェクトのコーディング規約によると思うのですが、アロー関数で「return」を省略できないか、
スプレッド演算子使えないか、オブジェクトのプロパティ名を省略できないか。
(ES6の書き方が使えないか？と言った方が良いかも...)
あとは閉じタグとか色々。
テストコードが足りているか
if文があれば、その分テストを書いているのか。など、テスト観点として足りているのかを確認する。
余分なテストを書いていないか
テストはあればあるだけ良いというわけではなく、意味のあるテストだけを書く必要がある。
なぜなら、テストコードも保守するコストがかかるから。
意味のないテストは書かない！
軽くテストしてみる
コードを一通り見終えると、「ここをこうしたらどうなるんだ？」「ここをこう動かしたらバグるんじゃね？」
みたいなところが出てくるので、そのような場合は実際にローカルで試してみる。
あとは、ユーザーが行わないような意地悪な動作とかも試してみる。
時間をかけすぎると、それはレビューではなくテストになってしまうので注意。
意識していること
分からないところは聞く
当たり前だが、分からないところを分からないまま承認ボタンを押してはいけない。
分からないところがあれば、質問のコメントをする。
プルリク内のやりとりだけでは難しい場合、slackや通話で直接聞く。
早めにレビュー着手する
PRがとんできたら、できるだけ早くレビューを行うようにしている。
これは、自分がそうされたら嬉しいからやっている。
即反応があるのは気持ちいい。
解決方法が分からなくてもコメントを残す
微妙なコードだけど、代替案が思いつかない！
という時は、代替案を考えるのに時間をかけ過ぎずに、微妙だと思っているコメントだけ残す。
実装者がサクッと直してくれるかもしれないし、他のレビュアーが代替案を提示してくれるかもしれない。
意見を押し付けない
自分の意見が100%正しいとは思ってはいけない。
一人でコードを書くなら好きな書き方をすれば良いが、チームの場合はそうではない。
それぞれ考えを持っていると思うのでリスペクトを持ってレビューをする。
意見が別れそうなコードがあれば、指摘というよりは提案のような形でコメントを残す。
こう書くのはどうでしょう？
ここは、こうだと思うのですがいかがでしょう？
など。
優しめな印象のコメントを残す。
責めるべきは、実装者ではなくコード。
「別に、あなたを責めているのではないですよ。」ということをわかって欲しいので、できるだけ優しい文章を心がける。
ここのコード、削除してください。
ではなく、
ここのコード、削除できますかね？
こっちの方がなんとなくだが、やさしい表現だと感じる。
回りくどいと言われればそうかもしれない。。
あとは、！つけるのは定番。印象がだいぶ違う気がしている。
ここのコード、削除できます！
お礼をする
指摘したコメントに対して、修正をしてもらえたら、その修正に対してお礼のコメントを残す。
これは正直、毎回できていることではない。
最低でも、「修正しました」というコメントにはいいねを押すようにしている。
指摘を受けるということ自体が、マイナスなイメージなので、
お礼をすることでそのマイナスがプラマイ0になってくれないかなー。という想いがある。
終わり
どう実装すれば良いかは教わるが、どうレビューすれば良いかは教わらなかったので、
最初はとりあえず自分が受けた指摘をレビュー観点として持ち、それを段々と増やしていった。
なので、レビュー観点といいつつ、自分がこれまで受けた指摘リストでもある。
あとは、他のレビュアーがどんなことをコメントしているのかを観て、それを自分の中に取り入れたりもした。
(優しめな文章とかお礼コメントとかは、完全に他のレビュアーから盗んだもの)
こんな感じで、あくまで自分流のレビュー観点なので参考までに。