はじめに

「読みやすいコードのガイドライン」を読んでチームに共有したい内容をまとめた。

なお本記事は前回の続きで今回はコードレビューにおけるレビューア側が意識するべき点について記載する。

1. レビューアの注意点1：レビュー実施時の基本原則

レビューによるコードの改善はレビューア・レビューイの共同作業であり、互いを尊重することが大前提
レビューの指摘はコード・仕様・プロセスといった、レビューア・レビューイの人格とは分離されたものを対象とするべき
レビューの重要な目的はコードベースの可読性を高く保ち、一定の生産性を維持することであり、逆にコードレビューに過剰な時間をかけることで生産効率を下げてしまっては本末転倒
重要な4つのポイント
- レビューを放置しない
- 問題のあるプルリクエストを拒否する
- 締切を意識しすぎない
- 「提案」以外のコメントを考慮する

1.1 レビューを放置しない

レビュー依頼を受けた時、いつまでに最初の返信をするかという期限を、チームのルールとして決めておく
- 例：勤務日において24時間以内に返答する
  - より優先度の高いタスクを抱えている場合は「すぐにはレビューできない」と返信する
  - 実際にはレビューできないにも関わらず、「レビューできる」と返信するのは避けるべき

1.2 問題のあるプルリクエストを拒否する

プルリクエストが大きすぎる場合や基本的な設計が間違えている場合、プルリクエストが根本的な問題を抱えていることもある。そのような状況でレビューを継続すると
- 効率的な改善ができなくなる
- 指摘の漏れも多くなる
- 結果として、レビューア、レビューイ双方の時間を浪費する
- 品質の低いコードがマージされる
このような事態を防ぐためにも、大きい問題のあるプルリクエストは一旦クローズしてもらい、新たに作り直してもらうことも必要。
プルリクエストを作り直してもらう場合、必ずフォローすること。
- どのように分割するか示す
- 設計に問題がある場合はスケルトンクラスを作ってもらい、改めて構造のみを対象にレビューをする
- 作り直しの前に設計について議論をしたり、デザインドキュメントを協力して作成する

1.3 締切を意識しすぎない

「忙しいから」や「締切が近いから」という理由で、レビューの品質を下げることは好ましくない
- そのような特例を繰り返すことで、低品質なレビューが常態化する
- 本当に忙しくなった時に更なる品質低下を招く
レビューのプロセスを急ぐ必要がある場合、最低限のこととして、急がなくてはならない理由を説明する
- 場合によっては仕様の変更などで対象できる
- 場合によっては仕様次回のリリースに延期できる
「絶対に期限までにマージしなくてはならない」という先入観にとらわれない
- レビューアが広い視野を持てるように気を付ける
どうしてもその場しのぎのコードを使ってでもマージを急ぐべき状況では？
- 後でどのようにコードを改善するか、大まかな方針の合意をとっておく
- その場しのぎのコードに対するコメントとテストを残しておく
- イシュー管理システム上で「将来のコード改善」のチケットを作成しておく

1.4 「提案」以外のコメントを考慮する

改善の提案を行う場合、具体的かつ明確な内容を書くことが好ましい
- 曖昧なコメントを書いてしまうと、レビューイはそのコメントをどう扱えばよいかわからず、精神的に負担がかかる可能性がある
- 明確な提案は、レビューイの取るべき行動が明確になり、レビューとコード更新の反復を減らすことができる
ただし、具体的な提案が必ずしも最善とは限らない
- 詳細を詰めようとするとレビューアの負担が大きくなる
- レビューイを教育するという観点では、あえて選択の幅を残すことも有効
- 具体的な提案のステップ
  - 問題点がないか確認を促す
  - 問題点を指摘する
  - 問題点とその再現方法や理由を提示する
  - 問題を解消するための手法をいくつか提示する
  - 問題を解消するための最適な手法を提示する
  - 問題を解消するためのコードを提案する
問題点の発見以前に、コードで不明な点がある場合は、レビューイに質問をする方が効率的
- 他の開発者も同様の疑問を持つ可能性が高いため、あらかじめその点を洗い出すという意味でも有用
対面の議論やチャットでのやり取りも選択肢としてはあるが、その場合は議論した内容や結論を要約してレビューコメントとして残しておく

レビューアの注意点2: コメントの内容

レビューで指摘する点は多岐にわたる
- 例
  - プログラミング原則、命名、コメント、状態、関数、依存関係
  - コーディングスタイル
  - テストコードや動作の確認手段
  - プルリクエストやコミットの大きさ・構造
  - タスクそのものの目的・範囲
  - コードの複雑性と達成したいことのバランス
  - バグやセキュリティ上の欠陥
  - パフォーマンスやフットプリントの変化
これらの中で静的解析ツールによって自動的に指摘できるものは可能な限りツールに任せるべき
「気がつかないうちにコードベースの可読性が悪化していた」という状況を避けるために、コードレビューを行う際は変更があったコードそのものだけでなく、その周辺コードや依存関係のあるコードも確認する必要がある！

感想

本記事で触れた4つのポイントはどれもコードレビューにおいて基本的なことだがとても重要なことだと思った。特に、レビューイ側の立場の場合、締切間近になると焦ってミスが増えるしまともな判断ができなくなることがあるため、レビューアは常に冷静に広い視野を持つべきである。これらの考え方をチーム全員が再認識することで、コードレビューの質が今までと違うものになるはずだ。まずはチーム内に共有する場を設けようと思う。

読みやすいコードのガイドライン（レビューア編）