悪いコードを憎んで人を憎まず！ プルリク送付前に心がけたいコードレビューのコミュニケーション術

「エンジニアは1人で黙々とコードを書くだけでなく、コミュニケーション力が必要」と言われることが多いと思います。コミュニケーション力は抽象的に扱われがちなスキルですが、場面と目的を明確にすれば具体的に学べると考えています。 今回対象とする場面はコードレビューです。多くの開発現場ではGitHubのPull Requestを使ってコードレビューが行われており、エンジニアにとって比重の高いコミュニケーションと言えるでしょう。

会社・チーム・コミュニティ・プログラミング言語などにより、コードレビューのコミュニケーションにもそれぞれ異なる文化があります。そのため、決まった形の正解はありませんが、若手エンジニアの方が開発チームに参加する際に参考にできるよう、広く使える考え方を解説します。

コードレビューを通して成長しよう

コードレビューの目的としてまず考えられるのは、バグを発見して事前に修正することです。このためにはコードを理解し、危ない箇所を見つけていけばいいでしょう。

しかし、コードレビューの目的はそれだけではないと思います。お互いの知識を交換したり、議論をしたりすることで、成果物の品質を高めつつ個人やチームのスキルアップを目指すことも目的の一つだと考えます。そのため、バグやミスを指摘するだけではなく、適切なコミュニケーションを通じて、チームワークを発揮することが必要です。

ここから、Pull Requestを出す側・レビューする側がそれぞれ「心がけるべきこと」について考えてみます。

Pull Requestを出す時

まずはPull Requestを出す時に心がけたい点を挙げていきます。

5W1Hを伝える

「○○を実装しました。」とだけコメントされたPull Requestはレビュワーを困らせてしまうかもしれません。いつまでにレビューすればいいのか、なぜこのコードが必要なのかといった情報が不足しているからです。コメントではコードの目的や重要度を整理して伝えましょう。

よくある方法ですが、5W1Hを使ってテンプレート的に記述することが有効です。それぞれ、下記のような内容をコメントするといいでしょう。

• When
  • いつまでにレビューしてほしいか
  • リリース予定日
• Where
  • 改修した範囲
  • バグの再現手順
• Who
  • どのステークホルダーが関係しているか
• What
  • ユーザ視点で何が変わるのか
  • 開発者視点で何が変わるのか
• Why
  • この方法を選んだ理由
• How
  • 実装方法の補足

このように整理することで効率良くレビューを進めることができます。コードを読み進める前に全体像をイメージでき、複数の選択肢の中からなぜこの方法を選んだのか、余計な質問を繰り返さずに済むというメリットがあります。

悩んでいることは意思表示しよう

設計や実装で悩んでいる部分は協力を求めましょう。スッキリしないまま微妙な方法で開発を進めてしまうと手戻りが大きくなることもあります。1人で解決できないケースは誰にでもあり、恥ずべきことではありません。

コメントでは自分が悩んでいる部分と、その理由を伝えましょう。

• 悩んでいる部分を伝える
  • この関数が綺麗に書けないのでアドバイスがほしい
  • この辺が何だか微妙だからアイデア募集
• なぜ悩んでいるか伝える
  • いくつかの方法があるがどれがベストなのか決められない
  • それぞれのメリット・デメリットを挙げて伝える

このように「チームワークで解決したい」と意思表示することで、他の開発メンバーも意見を出しやすくなり、協力しやすい状態を作れるでしょう。

Pull Requestは粒度を小さく、頻度を多くする

業務でチーム開発をする際はPull Requestの粒度を小さくし、頻度を増やすことをおすすめします。 1つのPull Requestが大きい場合、レビューするレビュワーの負担が大きくなります。差分が数100行以上ある場合は、かなりの集中力がないとバグを見逃してしまうリスクもあります。そのため小さくPull Requestを出してレビューの負担を減らし、少しずつ着実に進めていくのが得策です。数10行以内の差分であればレビュワーの負担も少なく、素早くマージまで進むことができると思います。

もう一つ、Pull Requestを出すスパンも短く設定するのがおすすめです。多くの場合はリリースまでの期日が決まっており、遅れが出た場合はチーム全体でカバーすることが往々にしてあるでしょう。こういったシチュエーションで、仮に3日に1回しかPull Requestを出さない担当者が急に病欠してしまったとすると、最大3日分の開発内容が見えず、他のメンバーが遅れをカバーすることは難しくなってきます。

一方、数時間の作業ごとにPull Requestを出していれば、進捗が他のメンバーにも見えるのでカバーは難しくありません。作りかけの状態であっても、全てのコードをWIP (Work in Progress) でPull Requestにしておくこともいい方法かもしれません。こうした「進捗と開発内容の可視化」はメンバー同士が成果に貢献するための第一歩になるでしょう。

レビューをする時

続いて、反対に“レビューをする時”に心がけたい点を考えてみましょう。ミスやバグといった危険な点や、可読性が低い、いわゆる「良くないコード」はレビューを通して改善する必要があります。しかし、気をつけてレビューを行わないと、レビュイー（実装者）を不用意に傷つけてしまう場合があります。

コードレビューは粗探しや指示ではない

大前提として認識したいのはレビューの目的はお互いに協力して良い成果物を作ることです。決して相手の粗探しや、上意下達の指示ではありません。良くないコードを改善するための指摘は必要ですが、主観的な表現ではなく客観的な事実を伝えましょう。例を挙げてみます。

この例では、具体的に何を改善すべきかが分かりません。「読みづらい」は主観的な感想ですし、「雑」「修正」といった言葉は必要以上に圧迫感のある表現です。真剣に取り組んだ成果物に対してこのような表現をされると、レビュイーは傷ついてしまうかもしれません。 同じ内容を言い換えてみます。

こちらの例ではまず褒めています。レビューは改善すべき点を挙げるだけの作業ではありません。良い点、学びになった点などは積極的に褒め、相手の仕事に対する感謝を伝えましょう。

あわせて「ネストが深い」という具体的な改善すべき点を挙げています。改善点に対して指示するのではなく変更を提案しています。同時に「メンテナンスしやすくなりそう」という改善の効果を伝えることでポジティブな表現になっており、レビュイーを傷つけず、モチベーションが出るようなコミュニケーションになったと言えるのではないでしょうか。

良い例のようなコメントを書くのは時間がかかります。私も以前、自分の時間や心に余裕がなくなってくると悪い例のようなレビューをしてしまっていました。悪い例のようなレビューはレビュイーを傷つけるだけでなく、実はレビュワー自身のモチベーションも低下させます。なぜかというと、粗探しをしてネガティブなフィードバックを繰り返していると、こうした態度が癖になってしまい、プロダクトやチームメンバーの悪い面ばかりに目がいくようになってしまいます。ひいては、プロダクトやチームメンバーに対するモチベーションが低下する、という悪循環に陥ってしまうからです。

コードレビューは毎日のコミュニケーションなので、そこでネガティブは言葉を使っていると自分の気持ちもネガティブになってきます。余裕がない時ほどポジティブなコミュニケーションを心がけていきましょう。

悪いのは理解しにくいコードであり、人ではない

レビューはコードに対して行うものであり、人に対して行うものではありません。人に対してコメントをすると攻撃になっていまいます。こちらも例を挙げてみます。

悪い例では相手の名前を出しています。繰り返しになりますが、レビューは人に対して行うものではありません。相手の名前を出すことは避けましょう。

良い例では「解決に向けて協力したい」と意思表示しています。レビューはお互いに協力して良いコードにするための作業です。人を攻撃するのではなく、協力して悪いコードを改善しましょう。

前項と合わせて、レビュー時の気遣いについて挙げました。このような配慮は非効率で時間の無駄と感じる人もいるかもしれません。たしかに、技術レベルの高いメンバー間や一緒に働いた期間の長いメンバー間、いわゆる「ツーカーの間柄」であれば、過度に気を使う必要はないでしょう。

しかし、多くの現場ではメンバー間の技術レベルも経験もバラバラで、中には自分に自信のないメンバーもいるでしょう。日々のコミュニケーションの多くを占めるコードレビューにおいて工夫や気遣いをすることで、お互いに協力しやすい雰囲気を作ってチームワークを高めていくことは大切だと考えます。

コメントの対応優先度を記そう

レビューでは重大なバグを発見することもありますし、些細なインデントミスを見つけることもあります。これらを全て同列に扱ってしまうと、どれが大事なのか分からなくなってしまいます。各コメントには以下のようなラベリングをし、変更をしないとレビューを通さない（対応必須）か、変更なしでもレビューを通す（任意）かを記すと良いでしょう。

• must：必ず変更すべき点
• imo：自分の意見や提案（in my opinion）
• nits：些細な指摘（nitpick、重箱の隅をつつくの意）

レベルの高い相手に対しては、自分の理解のために質問しよう

相手の技術レベルが高かったりレビュワーの経験が浅い場合、レビューは難しいと思われるかもしれません。そのような場合、レビュワーは積極的に質問をするのが良いでしょう。 レビュワーが分からないということはメンテナンスが難しいということです。コード自体に問題がない場合でも、難しいと感じた箇所には質問をして理解を深めるようにしましょう。質問を重ね、自身が成長することもチームへの貢献方法の一つです。

レビューは全てを受け入れなくてもいい

レビューを受けた後はコードに反映させていきます。改善できる点はもちろん反映させるべきですが、全てを受け入れなくても構いません。レビュワーが常に正しいわけではないですし、期日が迫っている場合は一部の可読性を犠牲にすることも必要な場合もあるでしょう。

議論をせずに全てのレビューを盲目的に受け入れることばかりが、コードの改善行動ではありません。客観的な事実に基づいてお互いに意見を出すことで学びが深まり、良い成果物を生むだけでなくスキルアップにもつながっていくはずです。

おわりに

コードレビューはエンジニアが毎日のように行うものであり、コミュニケーションの多くを占める場合もあります。Pull Requestを出す時・レビューをする時それぞれで工夫や気遣いをすることで、効率良く進めてコードの品質を向上させ、チームワークを高めていきましょう。

参考

ペアプログラミングを行えば、実装とコードレビューを同時に進められます。1つの画面を見ながら相談して進めることにより、協力して課題を解決する達成感があり、楽しく取り組めます。私は開発の一部のみで採用していますが、これからはもっと積極的に取り入れていきたいと考えています。

また、ペアよりも多い人数で行うモブプログラミングについても話題になることが多くなってきました。チームで集まって開発することでコミュニケーションコストが下がるメリットがあります。

対面でのコミュニケーションはツールを使う場合よりも容易なので、ペアプロやモブプロを活用すれば、本稿で挙げたテキストベースのコミュニケーションを前提とした課題は起きにくいかもしれません。

著者プロフィール

池田 惇（いけだ・じゅん）@jun_ikd

スマートフォンアプリエンジニア。iOS・Androidのアプリ開発を行いながらPMや開発チームリーダーを経験。エンジニアとして技術を学び続け、プロダクトマネジメントや人材育成でも活躍したい。アジャイル開発とオープンソースソフトウェアの活用を好む。

編集：薄井千春（ZINE）