コードレビューは無意味という話

主語がデカいというか、「すべてのコードレビューが無意味」はうそなんですが、「何も工夫のない原始的なコードレビューは非効率か逆効果」だとは思っています。プロセスとしてコードレビューを入れるなら、効果的に実践できるように指導するのがマネジメントの仕事でしょう。

何も工夫のない原始的なコードレビューと言っているのは、差分を見てレビューアが思いつきでコメントを入れるだけのものです。同じレビューアであっても言っていることが一貫しない。結果品質が安定しない。レビューアからしても毎回おなじことを言わなければならないと必要のない余計なストレスがたまる。

最悪なのはレビューアが品質にも納期にも責任を持っていない立場であるときだ。無責任に茶々入れられる結果に当然なるので、関係が悪くなる。レビューアもやれって言われて「見てやってる」って上からの気持ちだし、折り合わない。やらない方がマシだ。そのパッチの品質はちょっとは良くなるのかもしれないが、協力しあえない状態になる方が悪い。

安定した品質のものを余計なストレスなしにデリバリーできる状態にするというのは、マネージャーかアーキテクトと呼ばれる人がやるべきことなんですよ。プロセスを流れる成果物とプロセス自体があったとして、前者の品質には当然責任を彼らは負わなければならないが、後者を整えることにも彼らは責任がある。

コードレビューは、設計通りにコードが出来上がっているかと、動くかと、コードが規約通りになっているかどうかだけ見れば、それでいい。設計通りで動いて規約通りだったらどの変更は入れるべきです。レビューアの自分ルールなんて押し付けるべきではない。明文化されてもなければ聞いてもないことをあとから言われてやり直させられるのはストレスでしかない。

だから、コードレビューするなら前提してコーディングの規約がないといけない。例えばレベルの低いところだと、インデントがずれてるとか指摘するなら、ちゃんとインデントに関する規約を定めなければならない。さらに加えて Checkstyle の設定などを整備してスタイルチェックを自動でできる仕組みを整えなければならない。インデントがずれてると指摘だけして、コーディング規約を整えるわけでもなく、ビルドツールを整備することもなく、何度も同じことだけ言ってるのは、怠慢と主体性の欠如でしかない。

規約が十分でないこともあろう。規約がないところで指摘がしたくなったら、規約の方を育てる。規約が機械化されているなら設定をカスタマイズして、適用する。同じような箇所が他にもあるだろう。今の差分だけ直したって仕方ないともう直さないのか、今回の差分だけ直すのか、今回全部直すのが、バックログにいれるのか、入れるならどの優先度でやるのか、それは開発のマネジメントの判断である。人のコードに指摘するなら、自分の指摘がどれほどの影響があるか考えて指摘したい。

そのように育てられた規約とチェックツールは組織の財産です。規約で制限されたプログラミング言語は、不適切な機能や書き方を省いたプログラミング言語のサブセットであり、プログラミング言語を拡張して進化させたものと言って差し支えありません。ビルドが通るように作ったら勝手に適切な状態になる。そうしたらコードレビューなど要りませんよね。コードレビューはプログラミング言語の拡張の機会でしかない。開発組織をマネジメントするなら、こういう仕組みのほうにちゃんと投資したいし、投資してないところでは働きたくない。

ここまでやって、まだ規約では表現できない、問題のもっと良い解き方の議論などがレビューで行われることもあるだろう。これはコードレビューではないし、標準化もできないレビューアの能力が問われることだと思いますが、これこそ本質的でやるべきことだと思います。