コードレビューのベストプラクティス
コードレビューは絶対やらなければいけないものではありません。「コードレビューいらないくないですか？」と言われたこともあります。
しかし、多くの企業やオープンソースプロジェクトでは、おそらくコードレビューをしているのではないでしょうか？ コードを変更する際の健全性を保つ重要なプロセスと捉えている人は多いはずです。
この記事ではコードレビューのメリット・デメリットやコードレビューのやり方について触れていきます。
メリット
- コードの正しさをチェックできる
- 他者の視点からコードが理解できるものかチェックできる
- コードの書き方を統一できる
- レビュアー、レビュイーの双方がコードに責任をもつことになる
- 知識共有により成長やトラックナンバー[1]を増やせる
- コードレビューの内容は記録される
デメリット
- 否定され続けたレビュイーは自己肯定感が下がる
- レビューに時間を取られる
- 門番（承認権限者）の増加（アンチパターン）によりリリースが遅れる可能性がある
ベストプラクティス
コードレビューはやり方を間違えると、チームに亀裂を入れたり恐ろしい遅延を発生させる可能性があります。注意しなければいけないのは、それらはコードレビューが悪いのではなくやり方が悪いだけだということです。
ではどのようにコードレビューをすれば良いのでしょうか。
謙虚・尊敬・信頼の気持ちを忘れない
- Humility（謙虚）
- Respect（尊敬）
- Trust（信頼）
これらの頭文字を取ってHRTと言われたりします。
「〇〇してください」のような命令口調はあまり良くないです。「〇〇なので、〇〇のほうが良くないですか？」のように提案する形に変えたりなど、レビュアーもコード作成者もHRTを心がけましょう。
僕自身もまだまだ反省しなければいけないと感じる部分が多いです。
レビュアーとして淡々と指摘していたら「こわいです」と言われました。なのでなるべく絵文字を使うようにしています。
どれだけ優秀なエンジニアでもレビューされるのは不安ですし、指摘されれば少なからず精神的な負担があります。それらを和らげるためにもHRTはとても重要です。
コードの変更を待たずにLGTMにする
提案が終わったら変更前であってもLGTMを出します。コメントの修正提案を対応すれば、再レビューの必要なくマージして良いことを示すためです。
ちゃんと修正してくれるだろうという信頼からくる行動です。
テストコードによって修正の対応で致命的なバグがでないのも、信頼の助けになると思います。
レビュアーはコード作成者のアプローチを尊重すべき
自分と違う書き方という理由だけ指摘してはいけません。
自分と違う書き方というのは、お互いに学びのチャンスです。なぜそのような書き方をしたのか聞いてみましょう。（聞き方が嫌味にならないように注意）
自分が書いたコードはチームのもの
コードはコミットされた時点でチームのものになります。チームの為になる提案は歓迎するべきです。
コード作成者はコーディングのアプローチについて聞かれた場合、なぜそうしたのか、数年後も理解可能か、保守性が高いかなどを説明する心構えが必要です。
レビューは迅速に取り掛かる
24時間以内のフィードバックがベストです。もし24時間以内にフィードバックできないのであれば、なるべく早くレビューする旨を伝えるべきです。
レビューはまとめて返す
一つずつ指摘を返すのはやめましょう。修正してコミットしたあとで別の指摘を受けるのは、毒のようにじわじわと精神を蝕みます。
Githubなどではレビューボタンでまとめてコメントを返すようにしましょう。
レビュアーのコメントを無視しない
コード作成者はコメントを読み、対応後に解決済みにします。
レビュアーの提案をすべて受け入れる必要はないですが、賛同できないのであれば、その旨を伝えるべきです。そして、お互いに代案を出してから解決済みにしましょう。
変更は小さく書く
理想的なのは理解可能なコード量で一つの問題を解決するために、コードレビューをすることです。
小さな変更はレビュアーの負担を下げてくれます。そして、フィードバックも自然と速くなります。
小さな変更の具体的な数字は200行程度です。
とはいえ、全てが200行で収まることはなくて、新規で機能を作成した場合などは500行などになる場合もあります。そういった場合は、ブランチ運用で小さい単位の管理をする必要があります。
LGTMは1人がおこなう
複数人でレビューしてもいいですが、最終的にLGTMを与えるのは一人だけです。LGTMを与える人は保守性や可読性の知識、仕様把握ができる人であれば、だれがなっても構いません。
変更が小さければ問題がわかりやすくバグの発見もしやすいため、複数人でレビューする必要がなくなります。また、複数人でレビューしなければいけないのであれば、それはスケールしないということです。
それとは別に、コードの可読性や保守性を専門にレビューする人などで分担してレビューすることは可能です。
変更内容はちゃんと書く
コミットメッセージの1行目が特に大事で、どのような変更か、なぜ変更されるかを示す。このメッセージはプルリクのタイトルにもなる。
また、本文には実装の詳細を書くべきです。もしレビュアーに変更した理由が理解されていないのであれば、変更の説明が足りていないことを示唆しています。
また、コードレビュー中に決定されたことも書いておくと
可能な範囲で自動化する
仕様通りに実装されているかなどは、人間じゃないと判断できない部分が多いです。しかし、保守性があるか、可読性があるか、ケアレスミスのチェックなどは静的分析ツールを使うことで担保できます。
また、仕様部分に関してもテストコードを実行することである程度担保が可能になります。
自動化することで、レビュー速度を上げるだけでなく、レビュアーは重要な部分のチェックに専念できます。
さいごに
コードレビューは開発プロセスの中では割りと簡単に導入できて、実施しやすい部類です。それゆえに、特にやり方を考えることもなく、なんとなくでやっている部分でもあると思います。
この記事がコードレビューをする上での指針になれば幸いです。
-
メンバーがトラックに轢かれることでプロジェクトが進まなくなる人数。数字は大きいほど良い。 ↩︎
Discussion