あまり前職ではコードレビューをすることがありませんでしたが、今の職場ではやってます。
今、意識しているのは以下です。
- マージ（プル）リクエストのコード量を多くしない
- チェックアウトして変更分を見る
- 余裕を持った依頼をする
- レビューすることでコードへの理解を深める
プルリクエストのコード量を多くしない
差分が多過ぎてレビューができません
と言われてしまったことがあります。
ということを反省して、マージ（プル）リクエストをなるべく小さくするようにしてます。
また、コミットの粒度についても注意しています。
私は、あれこれやりたがる性格なので一つのコミットに複数の変更を入れてしまいがちです。
一つのコミットに一つの目的の修正にしてます。
ただ、プッシュ後に間違えたからコミットを修正することまではしてません。
無理にやると変な操作ミスをしてリモートをおかしくしたくありませんので。
チェックアウトして変更分を見る
レビューする側になるときはローカルにチェックアウトして、リクエストの変更分を見ながら確認するようにしてます。
わざわざチェックアウトするのは手間です。差分だけならばプルリクエストを見ればわかります。
ですが、差分だけを見てもわかりづらいです。
Gitでも必ずしもわかりやすい差分にならず、ブロック単位で大きく変わったように見えることがあります。
また、前後関係が読めないことがあります。
高性能なIDEを持っているので、その支援機能でtypoなどを拾ってくれるため、それを活かしてレビューすることが多いです。
余裕を持って依頼をする
もう時間ないんです。忙しいんです。すぐマージしてください。
それでは、依頼された側も致し方なくマージするしかないでしょう。
コードレビューに限らず、忙しいから、黙って承認してくれと強引に進めて、何か問題あっても、忙しかったから仕方ないと言う人がいます。
であれば、そんなプロセスはない方が良いでしょう。
本当にない方がいいプロセスはなくした方がいいですが、コードレビューは自分の見落としていた観点で指摘してもらえれば不具合が減ります。
やってもらった方がいいので、余裕を持って依頼をするように心がけます。
レビューすることでコードへの理解を深める
レビューというと指摘というイメージが強く、コードレビューだからこそわかる不具合というのもなきにしもあらずです。
それだけではなく、コードレビューにはこういうコードの修正を行なったという情報共有には良いと思ってます。
また、修正についての観点の認識合わせにもいいです。
こういう修正の方法でいいんですか？という確認をすることもあります。
おまけ
ちなみに、Forkwellという転職サイトでは「コードレビューの文化」という記載があったりします。
また、面接ではコードレビューの時のポイントは？という質問をされたことがあります。
