怖いですね… 今回はイライラさせたり、怖がらせたりしないようにコードレビューをするために個人的に気をつけていることを書こうと思います(それでも怖がらせてたらごめんね)。
いろいろなところで散々語られてることではありますが、自分の頭に置いてあることについて書きたいと思います。
前提
レビューする人・される人は使用言語やプラットフォーム・フレームワークにある程度のスキルを持っていて、新人とかではない。
コメントの相手はあくまでも仕事の関係の人の場合、もっと砕けてる関係の場合はラフにレビューし合うこともあります。
良くないパターン
イライラしたり、させたり、怖がったり怖がらせたり、大丈夫かな？思わせたりしがちなパターンと、そうしないためにどうすればいいか僕の考えを紹介します。
※ 基本的にはレビューする人側の話です。 こういう風にコード書けみたいなのは誰もが最初のうちは書けなくて当然だと思うので書きません。
どっちでもよくね？/ 結局どうすればいいの？/何が言いたいの？
これらは共通してレビューコメントが短すぎて説明不足な事が多いと思います。
例えば
「ここはXXXのようにすべきじゃないですか？」、「ここは○○○のほうが良いと思うのですが...」
というコメントなどです。
そう言われましても、「何故そうすべきなのか？」の理由がなければ判断ができません。適切に懸念を述べて、今のコードだとどのようなことが心配なのか？そしてどうすべきなのかをきちんと説明するコメントを心がけると良いです。
良いコメント例: 「このコードだとXXXの場合にYYYYのような問題が発生する可能性があるので○○○のほうがいいと思いますがどうでしょうか？」
あと多くの人はコード見たほうが理解早いと思うのでコードで示せる場合は仮想コードでもいいので例が書いてあるといいと思います。
一方レビューワー視点でなんとなく怪しいなと思って入るものの何故そうなのかがよくわからない場合も多いと思います。その場合でもちゃんとその旨を合わせてコメントすると良いと思います(正解が思いついて無くても懸念を共有することが大事です)。
例:「このコードだと具体的なパターンは思いつかないのですが、なんとなくメモリリークが発生しそうな気がするけど大丈夫ですか？」
など. そうすればちゃんと相手も調べて回答してくれると思います(回答してくれなかったら相手が悪い)。
熱量がわからない
最初に説明した説明不足なパターンとも似ているのですが、あるコメントがどのぐらいの熱量でかかれているのか分からず、これってコメント付いてるけど直したほうがいいの？と混乱することがあります。
この場合の対策としては文面からある部分に関して絶対に直してほしいのか、暇だったら直してほしいのかが分かるようにコメントすると良いと思います。
よく紹介されているのはコメントの先頭に絶対に修正必須だったらMUST やもうちょい考えてよみたいな気分だったら IMO とかつけるのが良いかと思います。
僕は絶対直してほしいときは MUST を使って、どっちでもいいかなと思うときは [直さなくてもどっちでもいいです] みたいなのを先頭に書いておくことが多いです。
全然レビューできてない
難しい変更内容のときにありがちですが、変更内容の中身についてのコメントが一切なく、些細な部分(コードフォーマット)だけにコメントが付くときがあります。
これはレビューを受ける側としてはちゃんとレビューできてるのかな？という不安があります。
この場合、レビュワーが気をつけることとしては大きく2つ以下のようなことがあります。
- 優れている部分には、その旨をコメントしてあげてちゃんと読んでいるということを伝える
- コードの難しい部分・自分の理解が不足していてレビューできない場合にはその旨をコメントする
- わからないことをわからないと言えることは大事です
最悪なパターンとしては、「なんか難しくてよくわからないけど、動いているからヨシ！」としてしまうパターンです。
行単位でのレビューはしてるけど、構造や設計についてレビューされていない
先程の例とも似ていますが、これはレビューする人のスキル不足な場合によく見かけます。
ネット上には各プログラミング言語の良い書き方が紹介されているので、多くの人はそういった知識はたくさん持っているので細かい書き方(シンタックスシュガーなど)についてはコメントができると思います。
一方、本質的なデータ構造、アーキテクチャ、過去の負債、会社の事情を踏まえて変更内容に問題がないのか？という視点でのコードレビューが全然なされないことがよくあります。
例えば「こういうか書き方すると XXX のようになって、これだと MVMO に書きにくくなってしまう」とか、「XXXX という理由から、A は B への依存を持つべきではない」みたいな複数のソースコードを跨がるような視点でのレビューです。
これは大変難しい技術だとは思うのですが、ある程度経験を積んだ人であればこういった広い視点でのレビューができて当たり前だと思うので頑張って勉強するほうが良いかと思います(自戒)。
イライラしない、させない簡単な方法
最後にいくつかの便利なトピックスを詳解.
- Lint、 Formatter、静的解析ツールを導入する
- レビューされる側は機械に言われたほうが何故かイライラしない
- レビューする側もいちいち細かいことを言わなくて済むので楽
- MUST じゃない指摘の場合はトゲトゲしない柔らかい感じの言葉遣いにする
- MUST な場合は、根拠とかをしっかり述べて相手に教えるつもりで丁寧に書く
- 「前は良かったじゃん」、「君もそう書いてたじゃん」みたいに以前のことを掘り出さない
- コードレビューは更に良くすることが目的だから、常に過去より良くしていこう
コードレビュー頑張ろう.
