コードレビューにおけるレビュアー側のアンチパターン

フューチャーアーキテクト（裏） Advent Calendar 2017 の25日目です。

tl;dr

• コードレビューが上手く回って無くてチームが疲弊して辛かったよ
• レビュアーの言い方を変えるだけで大体解決するよ
• 立場とかで例外を許さず、みんながレビューしてレビューされると良いよ

はじめに

あるプロジェクトでGitHubのPRベースでのコードレビューを導入をしました。いかんせんチーム開発が初めてレベルの新人さんが多く、何かと苦労しました。特にレビュイーに対して不効率な指摘はそのまま指示の不明確さに繋がり、チーム全体の開発生産性を下げるので、レビュアーはレビュイー以上に気を使う必要があると感じました。下手をすると、レビュイーのメンタルが弱って闇堕ちするので、チームメンバーの最も大人な人がメンタルケアしたりします。大人な人は大体がリーダー格なので、その人の時間が奪われると何かと開発現場が疲弊しちゃいますね。コードレビューってそんなに難しいものだっけと思ったりもしますが、反省の意味も込めて実際にあった事案を、アンチパターンとして4つ紹介します。

1. 意識高い系コメントパターン

事案

• 「もっとドメインを意識して書き直して下さい」
• 「これはサービスクラスでやる仕事じゃないです」
• 「この（クラス名|関数名|変数名）は責務を示していないので考え直し」

…と言った具合の、具体的な修正指示が含まれていない指摘です。対象がひよこードで激しくピヨピヨを主張していると、ついコメントが荒々しくなってしまうやつですね。レビュイーからするとドメインを意識して書いたつもりだし、多分サービスクラスに書くべきだと思って書いたり、命名は限度がありますがある程度の知識・経験が無いと上手いのが思いつかなったりするだけで、それなりにがんばって書いているので突き放すのは良くないです。そもそもスキル不足で機能要件を達成することに一杯一杯で、なんとか期日内に開発を終えてレビューに出したという裏事情かもしれません。

対応方法

• とにかく、具体的な修正指示を出しましょう。できれば例示を出してあげましょう。その上で、参考書籍を出してさらにレビュイーのスキルを向上させましょう。
• 注意ですが、初心者にあるべき論、つまりトップダウンからの思想的なフィードバックコメントをしても上手く成長しないことが多い気がします。あくまで、ボトムアップ的なコメントと添える程度が良いかなと
• 機能要件しかできていようであれば、そのポイントに絞ってレビューするか、ノイズが多く無理そうであれば、まずレビュイー本人にセルフレビューを依頼し、修正する時間をとってもらう

2. ムービングゴールポストパターン

1のような抽象度の高い指示出ししたときによく発生していたアンチパターンです。

事案

• 「以前、指摘した内容が全然直っていない。ついでにこれも直して」
• 「修正は良いけど、XXXな観点での考慮はした？」

…と言った、最初のレビューと修正を何度も繰り返しながら、雪だるま式に修正のスコープと内容が肥大化していくパターンです。せっかく指摘項目を修正しても、別の指摘項目が次から次へ出てきて、まるでゴールポストが移動するかのようです。

対応方法

• 最初の1,2回目で指摘しきれなかったものは、Issueなりに指摘内容を起票できないか考えましょう。アプリケーションの仕様を守れていないとか、非機能的にアウトで無ければ後回しにしたほうが良いでしょう
• 五月雨式に指示を出すと、レビュイーからするとタスクの工数感が見積もれなくなってしまいます。レビュイーはもっと多くの仕事をしているでしょうし、そうすると他の仕事にも悪影響を与えてしまいます。
• 単純に、レビュイーからするとモチベーションが上がらないでしょう。気が付かなかったのであれば仕方ないですが、なるべく指摘はまとめてくれないと、いつまで経っても達成感が得られません。1日で終わらせるつもりでがんばった仕事がそうでない時点で、かなりレビュイーに精神的な負荷をかけてしまいます。

3. これ読んで勉強し直してきてパターン

事案

• 「この記事読んで書き直して」と共有したのがポエム記事
• 「リーダブルコードを読んでから修正して」

フィードバックを諦め、自分が行ったインプットをメンバーに強要するパターンです。気持ちはわかりますが、これをやられるとタスクの進捗管理がメチャクチャになるので困るやつです。とにかく、まったくタスクが進まなくなります。

対応方法

• そもそも、リーダブルコードを読んでも直るかどうかは一種のカケです。本に書いてある内容であったとしても諦めて同じように指摘しましょう。そして書籍や参考サイトに書いてある内容がいかに優れているか匂わせ、興味を掻き立てましょう。押し付けてはダメです。逆に興味を失わせることになります
• 記事や書籍を読めば不要になる指摘であっても無駄にはなりません。レビュイーが記事や書籍を読んだ時の理解度がぐっと変わります
• また、ポエム記事は染みるように読めるためにはある程度のソウルジェムの汚れが必要です。まだピカピカのメンバーに共有しても効果は薄いでしょう

4. レビュアーがアンレビューパターン

事案

• 1~3のレビュアー「時間がないのでこの修正はmasterブランチに直Pushして良いですか？」

…で、本当にだれにもレビューされずにmasterにマージされた

対応方法

• 普段、レビュアーだった人がレビュイーにコードレビューされるフローは必ず用意した方が良いです。多くの人が指摘しているとおり、チーム内でスキルが低いメンバーがむしろレビューする側に回ることで、むしろ一番成長することが出来ます
• スキルが高く立場的にも上の人でも、コードの前では関係なくレビューする・される方が、お互いの立場の理解に繋がり、チーム内の雰囲気も良くなります
• 意外と、レビュアーのコードがクソなことも多く、若手のメンバーのレビューはしっかり見てくれるため予期しない不具合を見つけてくれることも多く、実際的に有益です

まとめ

• コードレビューはコミュ障なぼくらにとって、恒常的に発生するほぼ唯一のコミニケーションなので、大事に扱おう
• フワフワした発言ばっかりしても前に進まないので、目の前のピヨピヨしているコードを少しでも前に進めてあげよう
• 双方向にレビューし合おう