🐇

良いコードレビューとは何か:その2「指摘か?提案か?」

に公開

良いコードレビューとは何か:その1「良いレビューコメントの書き方」の続きです。

コードレビューにおけるレビュアーの悩みとして聞かれるのが、「レビューでどこまで指摘していいのか?」というものです。特に、コードの品質や設計に関する意見は、チーム内での合意形成が必要な場合が多く、個人の判断だけでは進められないこともあります。また、必ず従うべきルールと、あくまで提案に留まるべき点の線引きも難しいものです。

あまり細かく指摘しすぎると、レビューが重くなりすぎてしまい、逆に生産性を下げることもあります。一方で、重要な点を見逃してしまうと、後々のトラブルにつながる可能性もあります。では、どのようにしてこの“線引き”を判断すればよいのでしょうか?

ごくごく小さなチームにおいては、CTOやチームリーダーの経験が基準(線引き)になります。しかし、チームが大きくなったり、レビュアーが増えれば、個々人の感性に任せるのは難しくなります。

そこで、今回はレビューコメントの“線引き”をどう判断するかについて、いくつかのポイントを整理してみたいと思います。

指摘と提案の使い分け

レビューの基本は、指摘と提案の使い分けです。指摘は必ず修正が必要な点、提案は改善の余地がある点を意味します。この2つを明確に区別することで、レビュアーと開発者のコミュニケーションがスムーズになります。

指摘の例

指摘とは、いわば「Must」な事項です。必ず修正しなければならない点を指します。たとえば、以下のような内容が挙げられます。

  • 明確なバグ
    例えば、変数が未定義のまま使用されている場合
  • セキュリティ問題
    SQLインジェクションの脆弱性があるコード
  • パフォーマンス問題
    ループ内での不必要な計算が行われている場合
  • コーディング規約違反
    チーム内で合意されているコーディングスタイルに従っていない場合

これらの指摘事項を無視すると、アプリケーションの動作に重大な影響を及ぼす可能性があります。必ず指摘した上で、修正を待ってからマージするべきです。

提案の例

それに対して提案は、あくまで「Should」な事項です。必ずしも修正が必要ではないが、改善の余地がある点を指します。たとえば、以下のような内容が挙げられます。

  • コードスタイルの統一
    変数名の命名規則が一貫していない場合
  • 設計方針の見直し
    クラスの責務が多すぎる場合
  • コメントの追加
    意図がわかりにくい部分にコメントを追加すること

これらの提案は、チーム全体のコード品質を向上させるために有効ですが、必ずしも修正が必要なわけではありません。もちろん、チーム内で合意があれば、提案を「Must」に昇格させることも可能です。

ただし、コーディングスタイルは全員が一致するものではありません。自分の書き方を押し付けるのではなく、「あなたの意見」と「チームの方針」を明確に区別しなければなりません。アプリケーションの直接的な品質に影響しない部分については、あくまで提案として扱うべきです。

指摘と提案の線引き

指摘と提案の線引きは、レビュアーの経験やチームの文化によって異なることがあります。以下のポイントを参考に、どこまで踏み込むかを判断すると良いでしょう。

  • 影響度
    指摘がアプリケーションの動作やセキュリティに影響を与える場合は、必ず指摘するべきです。一方で、コードの可読性やスタイルに関する事項は、チーム内での合意が必要です。
  • 教育的価値
    開発者の学びにつながる場合は、積極的に指摘するべきです。特に新人や経験の浅い開発者に対して、指摘を通じて成長を促します。
  • 合意形成
    チーム内での合意がある事項については、指摘として扱います。逆に、個人の感性に基づく事項は、あくまで提案として扱うべきです。
  • チームの文化
    チームの文化や方針によって、線引きは変わります。たとえば、アジャイル開発では提案が多くなる傾向があり、ウォーターフォール開発では、指摘が多くなる傾向があります。

レビューの“深さ”はチームで揃える

レビューには“深さ”があります。つまり、どの程度までコードを詳細にチェックするかということです。この“深さ”は、チーム全体で揃えなければいけません。なぜなら、レビュアーごとに深さが異なると、コードの品質や一貫性にばらつきが生じるからです。

さらに深さのばらつきがあると、レビューを受ける方も“浅い”レビュアーに見てもらいたいと思うようになります。たとえば深いレビュアーが退勤したあとに、浅いレビュアーにレビュー依頼するといった具合です。これはチームの協力を損なうだけでなく、コードの品質にも悪影響を及ぼします。

レビューの浅い、深いというのは、どちらかに問題があるというわけではありません。チームで統一されていないことが問題なのです。

深掘りレビュー vs スピード重視レビュー

レビューの深さの違いは、主に以下の2つのスタイルに分かれます。

  • 深掘りレビュー
    • コードの細部までチェックし、品質を徹底的に向上させるスタイル
    • バグやセキュリティ問題を見逃さないため、時間がかかることが多い
  • スピード重視レビュー
    • コードの全体的な流れや設計を重視し、迅速にレビューを終えるスタイル
    • 細かい部分は見逃す可能性があるが、開発のスピードを優先する

この2つのスタイルは、チームの文化やプロジェクトの性質によって使い分けるべきです。これもまた、チーム内であらかじめ合意形成されているべきものです。

レビューで何を重視するかを決めるということは、レビューガイドラインを決めることと同義です。ガイドラインが決まることで、レビュアーもどういった部分をチェックすれば良いのかが明確になるので、負荷が軽減されます。

何を重視するかの合意がないと軋轢になる

レビューに関する合意がないと、チーム内で軋轢が生じることがあります。たとえば、あるレビュアーは細かい部分までチェックしたいと思っているのに対し、別のレビュアーはスピードを重視している場合です。このような状況では、レビューの結果に不満が残り、チーム全体の士気が下がる可能性があります。

また、レビュアー自身も自分の負担に対してストレスを感じますし、レビューされる方も毎回指摘事項が異なり、混乱します。こうしたストレスはお互いにとってフラストレーションになり、チームの精神的安全性を損なう結果になるでしょう。

スタイルは変えても良い

なお、レビューの深さやガイドラインはプロジェクト全体で統一する必要はありません。たとえば、特定のモジュールや機能に対しては深掘りレビューを行い、他の部分ではスピード重視のレビューを行うといった使い分けも可能です。アプリケーション層とインフラ層、データベース層でそれぞれ異なるレビューの深さを設定することもあります。これらは、システム修正による影響度によって異なります。

また、プロジェクトのフェーズによっても、レビューの深さは変わることがあります。たとえば、初期段階ではスピード重視で進め、後半で深掘りレビューを行うといったアプローチも有効です。初期の頃は、後々修正が重なる可能性が高く、初期状態で細かく見ていくのは非効率的です。徐々にコードが安定してきた段階で、深掘りレビューを行うことで、効率的にレビューが進められます。

判断に迷ったときどうするか

レビュアーであっても、完璧ではありません。また、開発担当者ほど、その修正内容に対して深いビジネス知識を備えているわけでもありません。したがって、レビューを行う際に「ここは指摘すべきか?それとも提案に留めるべきか?」と迷うこともあるでしょう。ペアレビューなどで自分に知識が乏しいとき「分からないからOK」とするのも、アプリケーション品質という考えれば間違っています。

そこで思い出してほしいのは、レビューはコミュニケーションだということです。指摘や提案は、100%正しくあるべきだと決めつけてはいけません。もしそうなると、レビュアーは様々な情報を調べて、確実に大丈夫だと思えることしか指摘できなくなります。

では、判断に迷ったときにはどうすればよいのでしょうか?以下のポイントを参考にしてみてください。

1. 自分の意見を明確にする

まず、指摘や提案をする前に、自分の意見を明確にしましょう。自分がなぜその点について指摘したいのか、どのような改善が必要だと考えているのかを整理します。これにより、相手に伝えるべきポイントが明確になり、コミュニケーションがスムーズになります。

また、それが「指摘」なのか「提案」なのかも明確にしましょう。たとえば、「このコードはバグがあるので修正してください」というのは指摘ですが、「このコードはこう書くともっと良くなると思います」というのは提案です。多くの場合、レビュアーは経験のあるエンジニアなので、書いている内容すべてが指摘に見えてしまいます。そうならないように、書いている内容が「指摘」なのか「提案」なのかを明確にすることが大切です。

2. 受け手の立場を考慮する

レビューを受ける側の立場を考慮することも重要です。特に、レビューを受ける側が新人や経験の浅いエンジニアである場合、指摘が厳しすぎると防御的になり、健全なコミュニケーションが難しくなることがあります。

そのため、指摘や提案を行う際には、相手の立場や経験を考慮し、適切なトーンで伝えることが大切です。指摘事項だけを端的に伝えると、命令的な印象を与えてしまいます。そこで、以下のような表現を使うと良いでしょう。

  • 「この部分、◯◯の面で少し気になります」
  • 「このコード、△△の観点から改善の余地があると思います」
  • 「このアプローチは、□□の観点からも考慮すると良いかもしれません」

3. 迷っていることを伝える

もし、あなたが指摘や提案に迷っている場合は、そのことを率直に伝えるのも一つの手です。たとえば、以下のような表現が考えられます。

  • 「この部分については、まだ迷っていますが、こうした方が良いかもしれません」
  • 「このアプローチについては、いくつかの選択肢があると思います。ご判断ください」
  • 「このコードについては、私の意見もまだ固まっていません。ご意見をお聞かせください」

この場合、相手からのフィードバックを受け、次のステップに進むのが良いでしょう。コミュニケーションを通じて、相手の意見を聞くことで、自分の考えも整理されますし、相手も自分の意見を尊重されていると感じるはずです。

なお。コミュニケーションが続きそうな場合には、レビューコメントではなくチャットや対面のディスカッションに切り替えるほうが良いです。非同期コミュニケーションは齟齬が生じやすく、誤解を招く可能性があります。

まとめ

コードレビューにおける指摘と提案の線引きは、レビュアーの経験やチームの文化によって異なります。しかし、重要なのは、これらの線引きを独りよがりで決めるのではなく、チーム全体で合意形成を図ることです。「指摘は必ず修正が必要」、「提案は改善の余地がある」などと明確に区別することで、レビュアーと開発者のコミュニケーションがスムーズになります。

そしてレビューで見るべきポイントや深さをチームで揃えましょう。それらを明文化すれば、ガイドラインのベースになります。ガイドラインを決めれば、チーム全体でのレビューの一貫性が保たれ、コードの品質も向上します。

おまけ:CodeRabbitの活用

CodeRabbitはAIコードレビューサービスで、GitHubやGitLabと連携してPRに対して自動的にレビューを実行します。開発者はCodeRabbitからの指摘を確認し、修正を行うことで、コードの品質を向上できます。最近リリースされたCodeRabbit for VSCodeを利用することで、ローカルでのレビューも「無料で」可能になりました。

ぜひ、CodeRabbitを活用して、コードレビューの効率化と品質向上を図ってください。

AI Code Reviews | CodeRabbit | Try for Free

CodeRabbit

Discussion