コードレビュアーを惚れさせる方法
コードレビューというと、レビュアーばかりに注目が集まります。しかしレビューにおいて、コードを書く開発者も、読む人と同じくらい重要です。レビューに向けてコードを準備する方法についてのガイダンスはほとんどなく、そのため書き手は、ただ知らないというだけでこのプロセスを台無しにしてしまうことがよくあります。
この記事では、書き手としてコードレビューに参加する際のベストプラクティスを紹介します。実際、この記事を読み終えるころには、あなたはレビューに出すコードの出し方があまりにも上手くなり、レビュアーは文字通りあなたに惚れてしまうでしょう。
でも、レビュアーに惚れられたくないのですが
惚れられてしまいます。諦めてください。死の床で「惚れられすぎた」と嘆いた人は、これまで一人もいません。
なぜコードレビューを改善するのか
コードレビューの技術を磨くことは、レビュアー、チーム、そして何よりもあなた自身のためになります。
より速く学べる：チェンジリストを適切に準備すれば、退屈なスタイル違反ではなく、あなたの成長につながる部分にレビュアーの注意を向けさせることができます。建設的な批判を大切にする姿勢を見せれば、レビュアーはより良いフィードバックを返してくれます。
周囲も成長する：あなたのコードレビューでの振る舞いは、同僚の手本になります。書き手としての効果的な習慣はチームに伝播し、彼らがあなたにコードを送ってきたときのあなたの仕事も楽になります。
チーム内の衝突を減らせる：コードレビューは摩擦が生じやすい場です。意識的かつ誠実に取り組めば、無用な衝突を最小限に抑えられます。
黄金律：レビュアーの時間を大切にする
当たり前に聞こえるかもしれませんが、書き手がレビュアーを自分専用の品質保証担当のように扱っている場面をよく見かけます。そうした書き手は、自分のミスを見つけたり、レビューしやすいようにチェンジリストを設計したりする努力をまったくしません。
チームメイトは毎日、限られた集中力を抱えて出社します。その一部をあなたのために使ってくれるということは、自分の仕事に使える時間が減るということです。だからこそ、その時間を最大限に価値あるものにするのが礼儀です。
レビューは、参加者同士が信頼し合うことで格段に良くなります。あなたがフィードバックを真摯に受け止めると分かれば、レビュアーはより力を入れてくれます。レビュアーを乗り越えるべき障害物のように見なすと、彼らが提供できる価値は限られてしまいます。
テクニック一覧
- まず自分のコードをレビューする
- わかりやすいチェンジリストの説明を書く
- 簡単なことは自動化する
- 疑問にはコード自体で答える
- 変更のスコープを絞る
- 機能的な変更と非機能的な変更を分ける
- 大きなチェンジリストは分割する
- 指摘には寛容に対応する
- レビュアーが間違っていても辛抱強く
- 対応内容は明示的に伝える
- 不足している情報は巧みに引き出す
- 迷ったらレビュアーに譲る
- レビューのラウンド間の遅延を最小限にする
1. まず自分のコードをレビューする
チームメイトにコードを送る前に、まず自分で読み返してみましょう。単に間違いを探すのではなく、初めて読む人の視点で想像してみるのです。どこで混乱しそうでしょうか。
私は、コードを書いてからレビューするまでに少し間を置くのが効果的だと感じています。多くの人は一日の終わりに変更を送り出しますが、そういうときこそケアレスミスを見落としがちです。翌朝まで待って、新鮮な目でチェンジリストを確認してからチームメイトに渡すようにしましょう。
できる限りレビュアーと同じ環境を再現しましょう。彼らが見るのと同じ差分ビューを使ってください。通常のエディタよりも、差分ビューで見たほうが単純なミスに気づきやすいものです。
完璧を求めすぎないでください。どうしても、消し忘れたデバッグコードや、除外するはずだった余計なファイルが混じったまま送ってしまうことはあります。そうしたミス自体は致命的ではありませんが、記録しておく価値はあります。自分の誤りの傾向に注意を払い、防ぐ仕組みを考えましょう。あまりに頻発すれば、レビュアーの時間を大切にしていないというメッセージになってしまいます。
2. わかりやすいチェンジリストの説明を書く
前職では、開発者向けメンターシッププログラムの一環で、シニアエンジニアと定期的に面談していました。初回の面談の前、彼は私に自分が書いた設計書を持ってくるように言いました。私がそれを渡しながら、プロジェクトの内容やチームの目標との関係を口頭で説明すると、彼は顔をしかめ、きっぱりと言いました。「今話したことは、すべて設計書の1ページ目に書いてあるべきだ」
彼の言うとおりでした。私は身近なチームメイトが読むことを想定して設計書を書いていましたが、他の読み手のことを考えていなかったのです。直接のチームメイト以外にも、パートナーチームやメンター、昇進委員会など、より広い読者がいます。彼ら全員が理解できる文書であるべきなのです。あの会話以来、私は自分の仕事の背景が伝わるように、どのように位置づけて説明するかを常に考えるようになりました。
チェンジリストの説明では、読み手が必要とする背景知識を要約しましょう。説明を書くときは特定のレビュアーを想定しているかもしれませんが、相手があなたと同じ文脈を持っているとは限りません。それに、他のチームメイトがこのチェンジリストを読むこともありますし、将来、変更履歴を振り返る人があなたの意図を理解できる必要もあります。
優れたチェンジリストの説明は、高い視点から何を達成する変更なのか、そしてなぜその変更を行うのかを説明します。
優れたチェンジリストの説明についてさらに深く知りたい方は、私の記事、「How to Write Useful Commit Messages」をご覧ください。
3. 簡単なことは自動化する
波括弧の位置が間違っていることや、変更が自動テストを壊したことをレビュアーに指摘してもらうようでは、彼らの時間を無駄にしていることになります。
自動テストは、チームの標準的なワークフローに組み込まれているべきです。レビューは、継続的インテグレーション環境ですべての自動チェックが通った後に始めるものです。
もしチームが残念ながら道を誤り、継続的インテグレーションへの投資を拒んでいるなら、自分でこれらのチェックを自動化しましょう。gitのpre-commitフックやリンター、フォーマッターを開発環境に追加し、コミットごとにコードが規約を守り、意図した挙動を保つようにしてください。
4. 疑問にはコード自体で答える
このやり取りの何が問題でしょうか？
書き手は私が関数を理解する手助けをしてくれましたが、次にこのコードを読む人はどうなるでしょうか。変更履歴を掘り起こして、過去のレビューでの議論をすべて読むべきなのでしょうか。さらに悪いのは、書き手が私の机まで来て口頭で説明するケースです。私の集中を妨げるうえ、その情報は他の誰にも共有されません。
レビュアーがコードの動きについて混乱を示したとき、解決策はその一人にだけ説明することではありません。すべての人にわかるようにする必要があるのです。
誰かの疑問に答える最良の方法は、コードをリファクタリングして混乱そのものをなくすことです。名前を変えたり、ロジックを組み替えたりして、より明確にできないでしょうか。コメントを書くのも一つの方法ですが、自然に自明になるコードには到底及びません。
5. 変更のスコープを絞る
スコープクリープは、コードレビューでよく見られるアンチパターンです。開発者は論理バグの修正を始めたのに、途中でUIの不具合に気づきます。「ついでに直してしまおう」と考えて別の修正も加えてしまうのです。しかし、これで話がややこしくなります。レビュアーは、どの変更が目的Aのためのもので、どれが目的Bのためのものかを判別しなければなりません。
最良のチェンジリストは、ただ一つのことだけをやるものです。変更が小さくシンプルなほど、レビュアーはすべての文脈を頭の中で保持しやすくなります。無関係な変更を分離すれば、複数のチームメイトに並行してレビューを依頼でき、変更が取り込まれるまでの時間も短縮できます。
6. 機能的な変更と非機能的な変更を分ける
スコープを最小化することの系として、機能的な変更と非機能的な変更を分けることが挙げられます。
コードレビューに不慣れな開発者は、このルールを破りがちです。2行だけ変更したつもりが、エディタが自動でファイル全体を再フォーマットしてしまうのです。開発者は何が起きたかに気づかないか、新しいフォーマットのほうが良いと判断して、そのまま送り出してしまいます。結果、2行の機能的な変更が、数百行に及ぶ空白だけの非機能的な変更の中に埋もれてしまうのです。
このチェンジリストの空白ノイズに埋もれた機能的な変更を見つけられますか？
ごちゃ混ぜのチェンジリストは、レビュアーにとって大きな負担です。空白だけの変更ならレビューは簡単です。2行の変更も簡単です。しかし、空白の海に埋もれた2行の機能的な変更は、退屈でうんざりする作業になります。
開発者はリファクタリングの際にも、不適切に変更を混ぜてしまいがちです。私はチームメイトがリファクタリングしてくれるのは大好きですが、挙動の変更と同時に行われるのは困ります。
このチェンジリストは挙動に一つの変更を加えていますが、リファクタリングの変更によってそれが隠れてしまっています。
あるコードにリファクタリングと挙動の変更の両方が必要な場合は、2〜3つのチェンジリストに分けるべきです。
- 既存の挙動を検証するテストを追加する（まだない場合）。
- テストコードはそのままに、プロダクションコードをリファクタリングする。
- プロダクションコードの挙動を変更し、それに合わせてテストを更新する。
ステップ2で自動テストに手を付けないことで、リファクタリングが挙動を保っていることをレビュアーに証明できます。ステップ3に進んだとき、レビュアーは挙動の変更をリファクタリングから解きほぐす必要がありません。あらかじめ分離しておいたからです。
7. 大きなチェンジリストは分割する
大きすぎるチェンジリストは、スコープクリープの厄介ないとこです。たとえば、機能Xを導入するために既存のライブラリAとBの仕様を変える必要があるとします。変更が少なければ問題ありませんが、こうした広範囲に及ぶ修正が増えすぎると、チェンジリストは巨大になってしまいます。
チェンジリストの複雑さは、触れるコード行数に応じて指数関数的に増大します。私の場合、プロダクションコードの変更が400行を超えたら、レビューを依頼する前に分割できないか検討します。
すべてを一度に変えるのではなく、まず依存関係を変更し、次のチェンジリストで新機能を追加できないでしょうか。機能の半分を今追加し、残りを次のチェンジリストで追加しても、コードベースを健全な状態に保てるでしょうか。
動作し、かつ理解可能な変更になるような部分集合を見つけるためにコードを分割するのは面倒ですが、より良いフィードバックが得られ、レビュアーへの負担も軽くなります。
8. 指摘には寛容に対応する
コードレビューを台無しにする最も手早い方法は、フィードバックを個人的に受け止めてしまうことです。多くの開発者は自分の仕事に誇りを持ち、それを自分自身の延長と捉えているため、これは難しいことです。レビュアーが無神経に人格攻撃のようにフィードバックを表現すれば、なおさらです。
書き手として、フィードバックへの反応を最終的に制御できるのはあなた自身です。レビュアーの指摘は、あなたという人間の価値を問うものではなく、コードについての客観的な議論として受け止めましょう。防御的になるほど、事態は悪化するだけです。
私はすべての指摘を役立つ学びとして解釈するようにしています。レビュアーが私のコードの恥ずかしいバグを見つけたとき、最初は言い訳をしたくなります。でも、そこで踏みとどまり、注意深く見てくれたことに感謝するようにしています。
レビュアーがあなたのコードの微妙なバグを見つけたときは、感謝を示しましょう。
意外かもしれませんが、レビュアーがあなたのコードの微妙な欠陥を見つけるのは良い兆候です。それは、あなたがチェンジリストをうまくまとめられている証拠です。フォーマットの乱れや分かりにくい命名といった明らかな問題がなければ、レビュアーはロジックや設計に深く集中でき、より価値のあるフィードバックが得られるのです。
9. レビュアーが間違っていても辛抱強く
ときには、レビュアーが完全に間違っていることもあります。あなたがうっかりバグのあるコードを書いてしまうのと同じように、レビュアーも正しいコードを誤解することがあります。
多くの開発者は、レビュアーの間違いに対して防御的になります。事実ですらない批判で自分のコードが侮辱されたと感じてしまうのです。
レビュアーが間違っていたとしても、それは依然として危険信号です。彼らが読み間違えたなら、他の人も同じ間違いをするのではないでしょうか。特定のバグが存在しないことを確認するために、読み手に異常なまでの注意力を求めていないでしょうか。
レビュアーが間違えたとき、相手が間違っていることを証明したい誘惑に負けないようにしましょう。
コードをリファクタリングしたり、コメントを加えたりして、より明らかに正しいことが分かるようにする方法を探しましょう。混乱の原因が分かりにくい言語機能にあるなら、専門家でなくても理解できる書き方に書き直してください。コードが明らかに正しいと分かるようにするのです。
10. 対応内容は明示的に伝える
私が指摘を渡し、相手が一部のフィードバックに対応してコードを更新したものの、返信が一切ないという状況によく出くわします。そうなると、宙ぶらりんな状態になります。他の指摘を見落としたのか、まだ作業中なのか。私が新たにレビューを始めれば、未完成のチェンジリストに時間を無駄にするかもしれません。かといって待っていれば、お互いが相手の出方を待つデッドロックが生じてしまいます。
チーム内で、どの時点でも誰が「バトンを持っている」のかが明確になるようにルールを決めましょう。書き手が修正作業中なのか、レビュアーがフィードバックを書いているのか。誰が何をすべきか分からずにプロセスが止まるような状況があってはなりません。これは、チェンジリスト全体へのコメントで主導権を渡したことを示せば、簡単に実現できます。
レビュアーに主導権を返すときは、チェンジリストにコメントして明示的に伝えましょう。
対応が必要な指摘には、それぞれ明示的に返信して対応したことを確認しましょう。コードレビューツールの中には、コメントを解決済みとしてマークできるものもあります。そうでない場合は、「Done（対応しました）」のようにシンプルなルールで各指摘に返信しましょう。指摘に同意できない場合は、なぜ対応しなかったのかを丁寧に説明してください。
ReviewableやGerritのようなコードレビューツールでは、書き手が特定の指摘を解決済みとしてマークする仕組みが用意されています。
レビュアーが費やした労力に応じて、返信の仕方を調整しましょう。何か新しいことを学べるように詳細な指摘を書いてくれたなら、単に「対応済み」とするだけでなく、丁寧に返信して感謝を示しましょう。
11. 不足している情報は巧みに引き出す
ときに、コードレビューの指摘は解釈の余地が大きすぎることがあります。「この関数は分かりにくい」といったコメントを受け取ったとき、「分かりにくい」とは具体的に何を意味するのかと疑問に思うでしょう。関数が長すぎるのか、名前が不明瞭なのか、それともドキュメントが必要なのか。
長い間、私は防御的に聞こえずに曖昧な指摘を明確にする方法に悩んでいました。直感的には「どこが分かりにくいんですか？」と聞きたくなりますが、それでは不機嫌に聞こえてしまいます。
あるとき、私がうっかり曖昧な指摘をチームメイトに送ったところ、彼は見事に角が立たない返し方をしてくれました。
どう直せばより良くなりますか？
この返答が気に入っているのは、防御的でなく、批判に対して開かれている姿勢が伝わるからです。レビュアーから不明瞭なフィードバックをもらったときは、いつも「どうすればより良くなるでしょうか？」といった形で返すようにしています。
もう一つの有効なテクニックは、レビュアーの意図を推測し、その想定に基づいて先回りしてコードを修正することです。「分かりにくい」といった指摘なら、自分のコードをもう一度見直してみましょう。たいてい、明確さを高めるためにできることは何かあります。修正することで、たとえ相手が意図していたものと違ったとしても、あなたが変更に対して前向きであることが伝わります。
12. 迷ったらレビュアーに譲る
テニスでは、相手のサーブがアウトかどうか迷ったときは、相手に有利に判定します。コードレビューでも同様の姿勢が求められるべきです。
全米テニス協会は、ライン判定の際に相手に疑わしきは有利に扱うことを選手に求めています。
コードに関する判断の中には、好みの問題であるものもあります。レビュアーが、あなたの8行の関数を5行ずつの2つの関数にしたほうが良いと考えても、どちらが客観的に「正しい」わけではありません。どちらのバージョンが良いかは意見の問題です。
レビュアーが提案をしてきて、どちらの立場にも同じ程度の根拠しかないときは、レビュアーに譲りましょう。二人のうち、新鮮な目でこのコードを読む感覚をよりよく理解しているのはレビュアーのほうです。
13. レビューのラウンド間の遅延を最小限にする
数か月前、私がメンテナンスしているオープンソースプロジェクトに、あるユーザーが小さな変更をコントリビュートしてくれました。私は数時間以内にフィードバックを返しましたが、相手はすぐに姿を消してしまいました。数日後に再度確認しましたが、まだ返事はありませんでした。
6週間後、その謎の開発者が再び現れて修正を提出してきました。努力には感謝しましたが、レビューのラウンド間の遅延によって、私の作業量は倍になってしまいました。コードを読み直すだけでなく、議論の記憶を呼び戻すために自分のフィードバックまで読み直さなければならなかったのです。もし1〜2日以内に対応してくれていれば、そんな余計な作業は必要ありませんでした。
6週間のブランクは極端な例ですが、チームメイトの間でも、長く不要な遅延をよく目にします。誰かがレビューのためにチェンジリストを送り、フィードバックを受け取ったのに、別のタスクに気を取られて1週間も後回しにしてしまうのです。
文脈を思い出すために失われる時間に加えて、中途半端なチェンジリストは複雑さを増大させます。何がすでにマージされ、何がまだ進行中なのかを全員が把握するのが難しくなるのです。未完成のチェンジリストが増えるほどマージの衝突も増え、誰もその解決を好みません。
一度コードを送り出したら、レビューを完了まで持っていくことを最優先にすべきです。あなた側の遅延はレビュアーの時間を無駄にし、チーム全体の複雑さを増やしてしまいます。
結論
次のチェンジリストをレビューに向けて準備するときは、自分でコントロールできる要素を意識し、それらを活用してレビューを生産的に導きましょう。レビューに参加する際は、進行を妨げたり労力を無駄にしたりするパターンがないか目を向けてみてください。
黄金律を忘れないでください。レビュアーの時間を大切にするのです。コードの面白い部分に集中できるようにすれば、レビュアーは質の高いフィードバックを生み出せます。コードを解きほぐさせたり、単純なミスを取り締まらせたりすれば、双方が損をします。
最後に、思慮深くコミュニケーションを取りましょう。単純な行き違いや軽率な一言でレビューが脱線するのは、驚くほど簡単です。他者の仕事を批評するときは感情が高ぶりやすいので、レビュアーが攻撃されたり軽んじられたと感じかねない落とし穴に注意してください。
おめでとうございます！ ここまで読み進めたあなたは、もうレビューを受ける側のエキスパートです。レビュアーはきっとあなたに惚れています。大切にしてあげてください。
参考文献
- How to Do Code Reviews Like a Human：書き手側の効果的なプラクティスを学んだところで、次はレビュアー側としてコードレビューを改善する方法を学びましょう。
イラスト：Loraine Yow 編集：Samantha Mason