コードレビューする側になって思うようになったこと

昔、コードレビューがなかなか通らずよく悩んでいた。

あれから時間がたって、最近はコードレビューをすることも多くなった。 レビューする側になったらなったで悩みが出てきたので、それについて簡単に書いてみる。

何の目的でどう修正したのかぱっとわからん

状況

PRの説明文を読んでも「何の目的で何を行ったのか」が読み取れない。 変更差分も大きく、コード差分からそれを理解するのも難しい。

希望

「何の目的でなにをしたのか」をまず書いてほしい。 付属の説明が必要な場合はその後に書いてほしい。 説明が複雑で長文になる場合は、問題を分けて一つずつPRに切り出すことで、問題を単純にしてほしい。

目的を達成する手段が適切ではない

状況

目的を達成する手段は色々あるが、目的達成に対して遠回りな手段を取っている。 遠回りなので、不要なリスクも抱えてしまっている。

希望

目的を実現する最短距離を目指してほしい。

実装コードに変更を入れることの危険度を認識できてない

状況

必要性も無いのにカジュアルに既存コードに大きい変更を入れる。 変更を入れたなら挙動に問題が無いことの担保が必要になるが、それが軽かったりもしくは無い。

希望

変更を入れるなら全力で「影響がない」ことの確認・担保は行ってほしい。 これをやろうと思うとだいたい面倒くさいから、余程の理由がないと既存コードに対して大きい変更を入れようとは思わず、影響範囲を適切に絞った変更になっていくと思う。

コードの変更理由について説明できてほしい。

状況

「なぜその手段を取ったのか」に理由がない場合がある。

希望

自分が書いたすべてのコードには、そう書いた理由を説明できてほしい。

仕組みは本来の用途で使ってほしい

状況

やりたいことが実現できるからといって、本来の用途でない使い方で仕組みを利用しようとする。

希望

いびつになっていつか必ずこまるのでやめてほしい。

信頼を勝ち得てほしい

状況

考慮されてなかった問題点を指摘することが毎度になると、その人に対する信頼度が下がって、レビュー時には「問題点があるはず」っていうモードになってしまう。掘ると気になる部分は出てくるので、細かい指摘まで発展してしまって、やりとりに時間がかかる。

希望

「ここ大丈夫？」って突っ込まれたら「〇〇なので検討済みです。大丈夫です！」って感じで打ち返して欲しい。 そうやって「この人はたぶんちゃんと色々考慮してくれてるから大丈夫だな」って思わせてほしい。 そうなると、レビュー指摘自体が減ってスムーズに行くようになる。

おわりに

書きながら、昔自分もそう思われていただろうな、、と思った。