CTOの杉本です。

「コードレビューが忙しくて開発の時間がとれない」というのは、ある程度役割が広がったエンジニアからよく上がる不満だと思います。

コードレビューはチーム開発で重要な活動ではありますが、「コードレビューで使う時間に対してどれだけの価値を出せるかを意識できていない」ことが原因の一つにあることが多い、と僕は思っています。

僕自身も立場上日々のコードレビューの負担が重く、うまく開発が進まないと感じていた時期がありました。時間対価値を意識して思い切ってやり方を変えてからは、コードレビューの負担の重さを感じることは少なくなりました。

今回は、僕個人が運用しているコードレビューへの考え方について書きます。

コードレビューの目的

そもそもコードレビューは何のためにやるのでしょうか？

僕が思うに、コードレビューの目的は「品質担保」と「開発者の成長」です。

最低限のコード品質・リリース品質を担保すること (品質担保)
開発者へフィードバックし将来的にレビューをする必要性をゼロに近づけること (開発者の成長)

この2点がコードレビューのメインの目的・コアの価値だと考えています。*1

2つの目的についてもう少し詳しく見ていきます。

品質担保

「最低限のコード品質・リリース品質を担保すること」での品質とは、次のような意味です:

読みやすく変更しやすいコードになっている (可読性・変更可能性)
自動テストがあり動作が確認できている
DBテーブル設計が今後の拡張をしやすいようになっている
パフォーマンス要件を満たしている

おそらく皆さんのイメージと沿うものだと思います。

ここで強調しておきたいのは、目指すのは「最低限の品質」であることです。良いコードであることは必要ですが、ベストのコード品質を目指す必要性はありません。

僕たち開発者の役割は、顧客にとっての価値が高いソフトウェアを届けることにあります。コードを最高に美しいコードにすることはそれ自体素晴らしいことではありますが、顧客価値に直接的には貢献しません。

最低限といっても基準は高く設定しており、読みやすく変更しやすいちゃんと動くコードにするルールを最低限守るという方針で開発を進めています。

開発者の成長

もう一つの重要な目的は、開発者の成長です。

理想的にはレビューの必要性がなくなり、コードレビューにかける時間が限りなく0になることを目指します。コードレビュー活動を通じて、開発者が次のような理想形に近づくために後押しします。

理想形: 開発者が自律的に品質の高いコードを書き、開発からリリースまでができるようになる

この理想形に近づくことで、開発チームがコードレビューのプロセスにかける時間が少なくなり、開発自体に使える時間が多くなります。また、実装からリリースまでのリードタイムが少なくなり、顧客に価値がより早く届くようになります。

なによりも、自分が書いたコードがそのまま最速でユーザーに届く体験ほど楽しいものはありません。高い生産性は開発者にとっての最高の自己実現だと僕は思います。

時間対価値を最大化する

「品質担保」と「開発者の成長」の2つの目的のために、僕たち開発者は日々レビューをしたりレビューを受けたりすることに時間を費やしています。つまり、コードレビューは開発組織にとっては「品質担保」と「開発者の成長」を目的として、開発チームの時間リソースを投入する投資的な活動だと言えます。

コードレビューに対しては「時間対価値を最大化する」ことを意識すべきだと思っています。なぜかというと、コードレビューはやることには価値がありますが、「たくさんやる」ことで価値はそれほど大きく増えないからです。

コードレビューに何時間もかけるよりは、同じ時間で開発者はコーディングなどの開発に時間を使ったほうが100万人以上のユーザーにインパクトがあるような改善ができます。

コードレビューは所詮1対1のコミュニケーションのため、レバレッジもあまり効きません。丁寧すぎるコードレビューをするよりは、ドキュメントなどに時間を使うほうが効率的です。そのため、コードレビューにかける時間はできるだけ無駄な時間がないように努力するようにします。

レビュワーとして意識すべきことは次のようになります:

自分の時間を「品質の向上」または「開発者の成長」の価値に繋げることを意識する
自分が使う時間を少なくすることを意識して、時間に対して生み出す価値が高くなるようにする

ここまでが基本的な考え方になります。ここからはもう少し具体的に普段意識していることを説明します。

以下ではレビュワーと実装者という言葉で説明することとします:

レビュワー: レビューをする人
実装者: レビューを受ける人

問題の指摘をする、解決策は提示しない

一般的に丁寧なレビューコメントは「Aという問題があります。たとえばXやYとするのではどうでしょうか？」という形になると思いますが、単に「Aという問題があります」と問題の指摘に集中するようにします。実装者から相談がない限りは解決策や対案の提示はしません。

なぜかというと次のような理由からです:

解決策までを考えると時間がかかりすぎる
実装者が成長する機会を阻害する

たとえば、次のようなコードがあるとします。

class Restaurant
  def process_data(data)
    ...
  end
end

このメソッド名は一見して意味がわかりにくいので、コメントしますが、このとき「メソッド名が汎用的で曖昧すぎるので、もっと具体的にしてほしいです」と問題のみをコメントします。より良いメソッド名をわざわざ考えて提案するようなことに時間を使いません。

理由1: 解決策までを考えると時間がかかりすぎる

ここで対案となる良い名前を提案しようとなると、レビュワーは実装内容・仕様内容を深く把握することや周辺のコンテキストを調査した上で、考えることに時間を費やさなければいけません。問題の指摘だけでなく、解決策までを考えると時間がかかりすぎます。実装内容・仕様内容に関しては、実装者のほうが理解度が高いはずなので、レビュワーがやったほうがいいような優位性は少ないことも多いです。

理由2: 実装者が成長する機会を阻害する

また、ここで「update_basic_information_from_data のほうがいいと思います」のように解決策を提案したとして、実装者が将来的に独力で良い命名を思いつけるようになるでしょうか？ならないと思います。良い命名を思いつくためには、時間をかけて調査をして考えるプロセスが重要なため、答えだけを得たところで学びや成長には繋がりません。丁寧・親切にしすぎることは、時に人の成長の機会を奪うことになります。

最近だと、ChatGPTなど共に手伝ってくれるためのツールの助けを得られるので、実装者が自力で解決できる問題の範囲も広がっています。相談を受けない限りは、敢えて問題を指摘するということに集中します。

問題は指摘するだけでも十分に価値があります。特に可読性などコード品質に関しては、実装者が問題を認識しづらいので、第三者が問題を指摘することで良くすることがずっと簡単になります。

バリューの薄いコメントはしない

レビュワーが意識すべきことは、レビューを受ける実装者側も返信して対応して修正するのに時間コストを費やすということです。相手の時間へのリスペクトを持ってコメントしなければいけません。

コメントする時間とそれに対応する時間を食う割に、得られる価値が少ない(バリューが薄い)と判断できる場合は、頭の中で思ったとしても敢えてコメントをしない選択をします。

nitpick はしない

コードスタイルの若干の乱れなど、相手が無視して良いコメントはしません。無視して良いと書いたとしても、心理的に相手は高い確率で対応したくなるため、相手の時間を奪うことになります。

もちろん、全体的にコードスタイルに沿っていないなど繰り返すパターンがあれば必ずコメントするようにはします。また、フォーマッターなど自動化できるものは自動化をしてチェックやコメントの必要性をないようにします。

IMO・弱いコメントはしない

IMO (In My Opinion) など、どちらでもいい程度の個人的な意見であればコメントすべきではないです。コードレビューは個人的な意見を表明する場所ではありません。

僕もついやってしまうことはありますが、できるだけコメントは絶対に修正すべき点のみにするようにしています。

深追い調査をしない

コードレビューをする際は深追い的な調査を行わないように気をつけています。実装者がいずれ問題を発見できるだろう場合は、レビュワー側は深い調査をせずに懸念のみをコメントするのみで十分だからです。

たとえば、コードを読んでいて「このSQLは遅すぎてパフォーマンス要件を満たしていないかも…」と思ったとします。レビュワーは手元でベンチマークをとったりしてコードを深く追ったり調査をしたりしてはいけません。実装者が検証環境で検証すれば、いずれ発見できる問題だからです。こういうときは確信度が低くても、単に「検証環境でパフォーマンスは計測しておいたほうがいいかもしれません」と懸念を伝えるか「パフォーマンス面が問題ないかベンチマークしてもらえますか？」と依頼します。

クリティカルポイントを最初に確認する

コードレビューする際に、最初で確認すべきなのは、全体の変更に影響がでるようなクリティカルな指摘事項がないかどうかです。クリティカルポイントがあれば、全体の実装が大きく変わることになり、個々の実装のレビューに費やしても時間が無駄になります。

たとえば次のような点がクリティカルポイントになりやすいです:

変更点が仕様や背景に沿っているかどうか
仕様や実装での考慮漏れがないかどうか
全体のデータ設計が妥当かどうか

まずクリティカルポイントがないかを確認をして、あればその時点で指摘を行ってレビューを止めます。

レビュー前の最低基準を設定する

レビュー前の最低基準を設定しておき、基準を満たしていない場合はレビューを始めないようにするのも時間を無駄にしないためには有効です。

レビュー前の最低基準を満たせていない例としては、次のような場合です:

必要なテストが書かれていない
一見してコードの体裁が整っていない
CIが通っていない

こういう場合はコードの中身を読まず、実装者にセルフレビュー依頼を返します。この状態でレビューをすると、レビューにかける時間が無駄になることが多いからです。特に、テストが書かれていないときは、動作していない可能性が高く、将来的にコードが変わりうるので、その時点でのコードのレビューにかけた時間が無駄になる可能性があります。セルフレビューをする習慣を促すほうが、長い目で見て開発者の成長に繋がります。

補足

この話には前提として、「開発者の技術レベルが十分に高く、自律して技術的な問題解決ができる」ことがあります。幸いなことに、今いるメンバーは技術力が高く自走できる人が多いため、こういった開発プロセス全体を任せるような思い切った方針が運用できています。

「不親切な印象になり雰囲気が悪くならないか？」と思われる方もいるとは思います。僕もたしかに…と一定思うことはあるのですが、期待値調整と関係値構築で軽減できる問題だと思っています。また、「A: 親切だが開発が遅い環境」と「B: 若干不親切だが開発が速い環境」のどっちで働きたいか？の問題ではあります。我々はスタートアップであり、僕なら迷いなく後者を選びます。