コードレビューというと、レビュアー側にばかり注目が集まる。しかしレビューにおいて、コードを書く側も読む側と同じくらい重要だ。レビューに向けてコードをどう準備するかについての指針はほとんどなく、だからこそ書き手は知らないがゆえにこのプロセスで失敗してしまうことが多い。

この記事では、あなたが書き手としてコードレビューに参加する際のベストプラクティスを紹介する。実際、この記事を読み終える頃には、レビューに出すコードの準備があまりにも完璧になっているので、レビュアーは文字通りあなたに惚れてしまうだろう。

でも、レビュアーに惚れられたくないんだけど

惚れられます。諦めてください。死の間際に「惚れられすぎた」と嘆いた人はいません。

なぜコードレビューを改善すべきなのか

コードレビューのスキルを磨くことは、レビュアーのためになり、チームのためになり、そして何よりあなた自身のためになる。

• より速く学べる：チェンジリストを適切に準備すれば、退屈なスタイル違反ではなく、あなたの成長につながる部分にレビュアーの注意を向けられる。建設的な批判を素直に受け止める姿勢を見せれば、レビュアーはより質の高いフィードバックを返してくれる。

• 周囲も成長する：あなたのレビュー作法は同僚の見本になる。書き手としての効果的な習慣はチームに伝染し、同僚があなたにコードを送ってくるときにもあなたの仕事は楽になる。

• チーム内の衝突を減らせる：コードレビューは摩擦のよくある原因だ。意識的かつ誠実に取り組めば、口論を最小限に抑えられる。

ゴールデンルール：レビュアーの時間を大切にする

当たり前に聞こえるかもしれないが、レビュアーを自分専用のQA担当のように扱う書き手をよく見かける。こういう書き手は、自分のミスを自分で見つけようとも、レビューしやすいようにチェンジリストを設計しようともしない。

チームメイトは毎日、限られた集中力を持って出社する。その一部をあなたのために割いてくれたなら、それは彼ら自身の仕事に使えない時間だ。彼らの時間の価値を最大化するのは当然の礼儀だ。

レビューは、参加者同士が信頼し合うと劇的に良くなる。あなたがフィードバックを真剣に受け止めると信じられれば、レビュアーはより力を入れてくれる。レビュアーを乗り越えるべき障害物とみなすことは、彼らが提供してくれる価値を自ら制限することになる。

テクニック一覧

1. まず自分のコードを自分でレビューする
2. 分かりやすいチェンジリストの説明を書く
3. 機械的にできることは自動化する
4. 疑問にはコード自体で答える
5. 変更のスコープを絞る
6. 機能的な変更と非機能的な変更を分ける
7. 大きなチェンジリストは分割する
8. 指摘には優雅に対応する
9. レビュアーが間違っていても辛抱強く
10. 対応は明示的に伝える
11. 足りない情報は巧みに引き出す
12. 迷ったらレビュアーに譲る
13. レビュー間のタイムラグを最小化する

1. まず自分のコードを自分でレビューする

チームメイトにコードを送る前に、まず自分で読んでみよう。単にミスを探すだけでなく、初めて読む人のつもりになってみる。どこで混乱しそうだろうか？

私は、コードを書いてからレビューするまでに一呼吸置くのが役立つと感じている。多くの人は一日の終わりに変更を投げがちだが、そういう時ほどケアレスミスを見逃しやすい。翌朝まで待って、新鮮な目でチェンジリストを見直してからチームメイトに渡そう。

できる限りレビュアーと同じ環境で見よう。彼らが見るのと同じdiff表示を使おう。通常のエディタで見るよりも、diff表示の方がくだらないミスに気づきやすい。

完璧である必要はない。どうしても、消し忘れたデバッグコードや除外するはずだった余計なファイルが混じったチェンジリストを送ってしまうことはある。そういうミス自体は致命的ではないが、記録しておく価値はある。自分の間違いの傾向に目を向け、再発を防ぐ仕組みを考えよう。頻発するようだと、レビュアーの時間を大切にしていないというメッセージになってしまう。

2. 分かりやすいチェンジリストの説明を書く

前職では、開発者メンター制度の一環でシニアエンジニアと定期的に会っていた。最初の面談の前に、彼から自分が書いた設計ドキュメントを持ってくるように言われた。私は彼に資料を渡しながら、このプロジェクトが何で、チームの目標とどう整合するかを説明した。メンターは顔をしかめて、ぶっきらぼうに言った。「今話してくれたことは、全部設計書の1ページ目に書かれているべきだ」

彼の言う通りだった。私はチームメイトが読むことを想定して設計書を書いたが、他の読み手を考慮していなかった。直接のチームメイト以外にも、連携先のチームやメンター、昇進委員会といった幅広い読者がいるのだ。彼ら全員が理解できるものでなければならない。あのやり取り以来、自分の仕事の背景をどう説明するかを常に考えるようになった。

チェンジリストの説明では、読者が必要とする背景知識を要約しよう。説明を書くときには特定のレビュアーを想定しているかもしれないが、相手があなたと同じ文脈を持っているとは限らない。そもそも他のチームメイトもこのチェンジリストを読む可能性があるし、将来変更履歴を振り返る人があなたの意図を理解できる必要もある。

良いチェンジリストの説明は、高い視点から、その変更が何を達成するのか、そしてなぜその変更をするのかを説明する。

優れたチェンジリストの説明についてさらに深く知りたいなら、私の記事、「How to Write Useful Commit Messages」を参照してほしい。

3. 機械的にできることは自動化する

波括弧の位置が間違っていることや、変更で自動テストが壊れたことをレビュアーに指摘してもらうようでは、彼らの時間を無駄にしている。

自動テストはチームの標準的なワークフローの一部であるべきだ。レビューは、継続的インテグレーション環境ですべての自動チェックが通ってから始めるものだ。

もしチームが迷走して継続的インテグレーションに投資しようとしないなら、自分でこれらのチェックを自動化しよう。gitのpre-commitフックやリンター、フォーマッターを開発環境に追加して、コミットごとにコードが規約を守り、意図した挙動を保つようにしよう。

4. 疑問にはコード自体で答える

このやり取りの何が問題だろうか？

書き手は私が関数を理解するのを助けてくれたが、次にこのコードを読む人はどうなるだろう？変更履歴を掘り返して過去のコードレビューの議論をすべて読むべきだろうか？さらに悪いのは、書き手が私の机まで来て口頭で説明するケースで、それは私の集中を妨げるうえ、他の誰もその情報にアクセスできないことを確実にしてしまう。

レビュアーがコードの動作について混乱を示したとき、解決策はその一人にだけ説明することではない。全員に分かるようにする必要がある。

誰かの疑問に答える最良の方法は、コードをリファクタリングして混乱自体をなくすことだ。名前を変えたりロジックを組み替えたりして、より明確にできないだろうか？コードコメントも許容できる解決策だが、自然に自己説明するコードには明確に劣る。

5. 変更のスコープを絞る

スコープクリープはコードレビューでよく見られるアンチパターンだ。開発者がロジックのバグを直し始めたとき、ついでにUIの不具合に気づく。「せっかくだから」と考えて、別の問題も直してしまう。だがこれで話がややこしくなる。レビュアーは、どの変更が目的Aのためで、どれが目的Bのためなのかを解きほぐさなければならなくなる。

最高のチェンジリストはただ一つのことだけをやる。変更が小さくシンプルなほど、レビュアーはすべての文脈を頭の中に留めやすくなる。無関係な変更を分離すれば、複数のチームメイトにレビューを並行して依頼でき、変更が反映されるまでの時間も短縮できる。

6. 機能的な変更と非機能的な変更を分ける

スコープを最小化することの当然の帰結が、機能的な変更と非機能的な変更を分けることだ。

コードレビューに慣れていない開発者は、このルールをよく破る。2行だけ変更したつもりが、エディタが自動でファイル全体を再フォーマットしてしまう。開発者は自分が何をしたかに気づかなかったり、新しいフォーマットの方が良いと判断したりする。そして、数百行の非機能的な空白変更の中に埋もれた2行の機能的な変更を送り出してしまう。

ごちゃ混ぜのチェンジリストはレビュアーに対して非常に失礼だ。空白だけの変更はレビューしやすい。2行の変更もレビューしやすい。だが、空白変更の海に埋もれた2行の機能的な変更は、うんざりするほど面倒で腹立たしい。

開発者はリファクタリングの際にも、不適切に変更を混ぜがちだ。チームメイトがコードをリファクタリングしてくれるのは大好きだが、挙動を変えながらリファクタリングされるのは勘弁してほしい。

もしコードにリファクタリングと挙動の変更の両方が必要なら、2〜3つのチェンジリストに分けるべきだ：

1. 既存の挙動を検証するテストを追加する（まだなければ）。
2. テストコードはそのままに、本番コードをリファクタリングする。
3. 本番コードの挙動を変更し、それに合わせてテストを更新する。

ステップ2で自動テストに手を付けないことで、リファクタリングが挙動を保っていることをレビュアーに証明できる。ステップ3に進んだとき、あなたが事前に分離しておいたおかげで、レビュアーは挙動の変更とリファクタリングの変更を解きほぐす必要がなくなる。

7. 大きなチェンジリストは分割する

巨大なチェンジリストはスコープクリープの醜い親戚だ。たとえば、機能Xを導入するために既存のライブラリAとBのセマンティクスを修正しなければならないとする。変更が少なければ問題ないが、こうした広範囲にわたる修正が増えすぎると、チェンジリストは膨大になってしまう。

チェンジリストの複雑さは、触れるコード行数に伴って指数関数的に増大する。私の場合、本番コードで400行を超えたら、レビューを依頼する前に分割できないか検討する。

すべてを一度に変えるのではなく、まず依存関係を変えてから次のチェンジリストで新機能を追加できないだろうか？機能の半分を今追加し、残りを次のチェンジリストで追加しても、コードベースを健全な状態に保てるだろうか？

動作して理解可能な変更になるような部分集合を見つけるためにコードを分割するのは面倒だが、より良いフィードバックが得られ、レビュアーへの負担も軽くなる。

8. 指摘には優雅に対応する

コードレビューを台無しにする最も早い方法は、フィードバックを個人的に受け止めることだ。多くの開発者は自分の仕事に誇りを持ち、それを自分自身の延長のように感じているため、これは難しい。レビュアーがフィードバックを個人攻撃のように無神経に表現すれば、なおさらだ。

書き手として、フィードバックへの反応を最終的にコントロールするのはあなた自身だ。レビュアーの指摘は、あなたの人間としての価値ではなく、コードについての客観的な議論として受け止めよう。防御的に対応しても事態は悪化するだけだ。

私はすべての指摘を有益な学びとして解釈するようにしている。恥ずかしいバグをレビュアーに見つけられたとき、最初は言い訳をしたくなる。だが、そこで踏みとどまって、注意深く見てくれたことに感謝するようにしている。

意外かもしれないが、レビュアーがコードの微妙な欠陥を見つけてくれるのは良い兆候だ。それは、あなたがチェンジリストをうまくまとめられている証拠だ。フォーマットの乱れや分かりにくい名前といった明白な問題がなければ、レビュアーはロジックや設計に深く集中でき、より価値のあるフィードバックが生まれる。

9. レビュアーが間違っていても辛抱強く

時にはレビュアーが完全に間違っていることもある。あなたがうっかりバグのあるコードを書いてしまうように、レビュアーも正しいコードを誤解することがある。

多くの開発者は、レビュアーの間違いに対して防御的に反応する。そもそも事実ですらない批判で自分のコードが侮辱されたと受け取ってしまうのだ。

レビュアーが間違っていたとしても、それは依然として警告サインだ。彼らが読み間違えたなら、他の人も同じ間違いをするのではないか？特定のバグが存在しないことを確認するために、読者は異常なまでの注意を払わなければならないのだろうか？

コードをリファクタリングしたり、コメントを追加したりして、コードをより明らかに正しいものにする方法を探そう。混乱の原因が難解な言語機能にあるなら、専門家でなくても理解できる方法で書き直そう。

10. 対応は明示的に伝える

私はよく、誰かに指摘を渡したあと、相手がフィードバックの一部だけに対応してコードを更新し、何も返信を書かないという状況に遭遇する。そうなると曖昧な状態に陥る。他の指摘を見逃したのか、まだ作業中なのか？私が新たにレビューを始めれば、未完成のチェンジリストに時間を無駄にするかもしれない。かといって待っていれば、お互いに相手が続けるのを待つというデッドロックを生みかねない。

チーム内で、どの時点でも誰が「バトンを持っている」のかが明確になるような規約を決めておこう。書き手が修正作業中なのか、レビュアーがフィードバックを書いているのか、どちらかだ。誰が何をすべきか分からずにプロセスが止まるような事態は決してあってはならない。これは、チェンジリスト全体へのコメントで主導権を渡すタイミングを示せば簡単に実現できる。

対応が必要な指摘にはすべて、対応したことを確認できるよう明示的に返信しよう。コードレビューツールの中にはコメントを解決済みとしてマークできるものもある。そうでなければ、「対応しました」のようにシンプルな規約に従って各指摘に返信しよう。指摘に同意できない場合は、なぜ対応しなかったのかを丁寧に説明しよう。

レビュアーの労力に応じて返信を調整しよう。何か新しいことを学べるようにと詳細な指摘を書いてくれたなら、単に完了とマークするだけで終わらせない。丁寧に返信して、その労力への感謝を示そう。

11. 足りない情報は巧みに引き出す

コードレビューの指摘は、解釈の余地を残しすぎることがある。「この関数は分かりにくい」といったコメントを受け取ったら、「分かりにくい」とは正確に何を意味するのかと疑問に思うだろう。関数が長すぎるのか？名前が不明瞭なのか？もっとドキュメントが必要なのか？

長い間、私は防御的に聞こえずに曖昧な指摘を明確にするのに苦労していた。本能的には「何が分かりにくいのですか？」と聞きたくなるが、それは不機嫌に響いてしまう。

あるとき、私がうっかり曖昧な指摘をチームメイトに送ったところ、彼は見事に肩の力を抜かせるような返しをしてくれた：

どんな変更があれば分かりやすくなりますか？

この返答が好きなのは、防御的でなく批判に対してオープンであることを示しているからだ。レビュアーから不明瞭なフィードバックをもらったときは、いつも「どうすればより良くなりますか？」といった形で返すようにしている。

もう一つの有用なテクニックは、レビュアーの意図を推測して、その想定に基づいて先回りしてコードを修正することだ。「分かりにくい」といった指摘なら、自分のコードをもう一度見直してみよう。大抵は、明確さを高めるために何かできることがある。修正を加えることは、たとえそれが相手の意図と違ったとしても、あなたが変更を受け入れる姿勢であることをレビュアーに伝える。

12. 迷ったらレビュアーに譲る

テニスでは、相手のサーブがアウトだったか確信が持てないときは、相手に有利になるように判定する。コードレビューでも同じ心構えが求められる。

コードに関する決定の中には、個人の好みの問題であるものもある。レビュアーがあなたの8行の関数を5行の関数2つに分けた方が良いと考えたとしても、どちらが客観的に「正しい」わけでもない。どちらのバージョンが良いかは意見の問題だ。

レビュアーが提案をしてきて、双方の主張を裏付ける根拠がほぼ同等なら、レビュアーに譲ろう。二人のうち、コードを初めて読むのがどんな感じかをよりよく理解しているのは彼らの方だ。

13. レビュー間のタイムラグを最小化する

数ヶ月前、私がメンテナンスしているオープンソースプロジェクトに、あるユーザーが小さな変更をコントリビュートしてくれた。私は数時間以内にフィードバックを返したが、相手はすぐに姿を消してしまった。数日後に再度確認しても、依然として返事はなかった。

6週間後、その謎の開発者が再び現れて修正を提出してきた。努力には感謝したが、レビューラウンド間の遅延で私の作業量は倍増した。コードを読み直すだけでなく、議論の記憶を取り戻すために自分のフィードバックまで読み直さなければならなかったのだ。1〜2日以内に対応してくれていれば、そんな余計な作業は必要なかった。

6週間の停滞は極端な例だが、チームメイト間でも長く不要な遅延はよく見かける。誰かがレビューのためにチェンジリストを送り、フィードバックを受け取ったあと、別のタスクに気を取られて1週間も後回しにしてしまうのだ。

文脈を思い出すために失われる時間に加え、中途半端なチェンジリストは複雑さを増す。何がすでにマージされ、何が進行中なのかを全員が把握するのが難しくなる。部分的に完了したチェンジリストが増えるほどマージコンフリクトも増え、そんなものを直したい人はいない。

一度コードを送り出したら、レビューを完了まで推し進めることを最優先にすべきだ。あなた側の遅延はレビュアーの時間を無駄にし、チーム全体の複雑さを増大させる。

おわりに

次のチェンジリストをレビューに向けて準備する際には、自分でコントロールできる要素を意識し、それらを活用してレビューを生産的に導こう。レビューに参加する中で、進行を停滞させたり労力を無駄にしたりするパターンがないか目を向けよう。

ゴールデンルールを忘れないでほしい：レビュアーの時間を大切にすることだ。コードの面白い部分に集中できるようにすれば、レビュアーは質の高いフィードバックを生み出せる。コードを解きほぐさせたり、単純なミスを取り締まらせたりすれば、双方が損をする。

最後に、思慮深くコミュニケーションを取ろう。単純な行き違いや軽率なコメントでレビューが脱線するのは驚くほど簡単だ。他人の仕事を批評するときは感情が高ぶりやすいので、レビュアーが攻撃されたり軽んじられたりしたと感じる落とし穴を意識しよう。

おめでとう！ここまでたどり着いたあなたは、もうレビューを受ける専門家だ。レビュアーはきっとあなたに惚れているはずなので、大切に接しよう。

参考文献

• How to Do Code Reviews Like a Human：書き手側の効果的なプラクティスを学んだところで、次はレビュアー側としてコードレビューを改善する方法を学ぼう。

イラスト：Loraine Yow、編集：Samantha Mason。