「気に入らないコード」をレビューする際にRed Hatのエンジニアはどうしているのか：10個のヒントとは？

プロジェクトメンテナーの立場にあるとき、提出されたコードが何らかの理由で気に入らない場合はどうしたらよいだろうか。Red Hatのソフトウェアエンジニアが、コードレビューを行うに当たって念頭に置くべき10のヒントを解説した。

[＠IT]

デビッド・ロイド氏

　Red Hatでソフトウェアエンジニアを務めるデビッド・ロイド氏は2019年7月8日（米国時間）、コードレビューを行うに当たって念頭に置くべき10のヒントを同社の開発者向け公式ブログで解説した。プロジェクトメンテナーの立場にあるとき、提出されたコードが何らかの理由で気に入らない場合に役立つ指針だ。コントリビューター側としても参考になる。

　これらのヒントは、客観的で的を射たレビューを行い、プロジェクトとその参加者を前進させるという観点からまとめられている。

（1）反対意見を質問に言い換える

• 悪い例：「この変更を実行すると、『XXX』が不可能になります」
• 良い例：「どうすれば、あなたの変更を受け入れるとともに『XXX』を行えるでしょうか」

（2）大げさな表現を使わない

　望ましい結果になるように、簡潔に懸念事項を述べたり、質問したりする。

• 悪い例：「この変更は、パフォーマンスをメチャクチャにしてしまう」
• 良い例：「『X』を行うと、既存の『Y』が遅くなる可能性がありそうです。そうならないことを示すデータを測定しましたか／集めましたか」
• より良い例（もし時間があるなら）：「当面、『X』が『Y』を遅くしないことを確認するデータを集めましょう」
• 別の良い例：「この変更を行うと、シングルループ『O（n）』が二重にネストされたループ『O（n2）』になります。これはパフォーマンスに影響しないでしょうか」

（3）悪意を表に出さない

　心に留めておいた方がよい意見もある。礼儀正しくできないのであれば、関与しない方がよい。

• 悪い例：「この変更は悪手であり、全てを台無しにすると思う」
• 悪い例：「こんなコードを書いても、ソフトウェアエンジニアがあなた自身に合ったキャリアパスだと確信できますか」

（4）建設的に関わる

　問題解決の方法について、提出されたコードとは異なるアイデアがあったとしても、建設的に関われば、その2つの選択肢より、さらに良い解決策が見つかるかもしれない。

• 悪い例：「この変更はお粗末ですね。私のやり方の方がよいでしょう」
• 良い例：「『XXX』の場所で似たような変更を施したことがあります。アイデアを比べたり、組み合わせたりできるかもしれません」
• 良い例：「ちょうど似たような変更を進めていますが、『ZZZ』という理由で『X』をすることを選びました。あなたはなぜ『Y』を選びましたか」

（5）誰もが自分と同じ経験や前提知識があるとは限らないことを念頭に置く

　明らかなことを発言してもよいが、恩着せがましく言ったり、小言を挟んだりしない。

• 悪い例：「これが明らかに間違っていることが分からないのでしょうか」
• 良い例「これは正しくありません。『X』が『Y』の場合に、nullポインタ例外を引き起こすからです」

（6）明らかではない事柄があったとき、その複雑さを単純化しない

　自分にとって明白だと思えることがあっても、相手にはとってはそうではないかもしれない。代替アプローチを提案したり、有効な例を挙げたりすると、理解の共有に役立つ。

• 悪い例：「裏技があるのになぜ使わないのでしょうか」
• 良い例：「裏技が使えるかもしれません。そうすれば、この部分が簡単になります（『X』の例を見てみましょう）」

（7）相手を尊重する

　提出されたコードが品質の最低基準を満たしていないこともある。それを指摘しても問題はないが、その際に相手を尊重してもコストはかからない。

• 悪い例：「これは間抜けが書いた間抜けなコードですね」
• 良い例：「あなたの貢献には感謝します。だが、このコードは受け入れられません。複数の問題があるからです」
• 良い例：「あなたの提案には複数の問題があります。一歩下がって、代わりとなるユースケースについて話してもよいと思います。そうすれば私たちが進む道を探すのに役立つはずです」

（8）希望と自分の時間のバランスをとる

　提出されたコードが長すぎて適切にレビューできない場合、作成者にそれを知らせても問題はない。

• 悪い例：「コードが長すぎるため、マージしません」
• 悪い例：何もコメントせず、コードが取り下げられるまで無視する
• 良い例：「複数の小規模な変更に分割していただけませんか。コードレビューの時間があまり取れないので、このコードは長く、複雑すぎます。1回で全てを見ることはできません」

（9）丁寧に頼む

　「〜していただけますか」と言葉を加えて丁寧に頼めば、こちらが作成者の時間についても尊重していることが伝わる。

• 良い例：「空白についての変更は、別のプルリクエストに分けていただけますか」
• 良い例：「コードが読みやすいように、変数定義を統一していただけますか」

（10）対話を始める

　以上のことを念頭に置いても、まだ受け取ったコードが気に入らず、理由も分からない場合は、我慢しなければならないのかもしれない。だが、「私はこのコードが良いと思えないが、理由が分からない。コードについて話すことはできますか」と、作成者に対話を呼び掛けるのは問題ない。

　こうした対話を求めるのは理にかなっており、レビュワーとしての誠実さの表れでもある。対話によって少し時間はかかるかもしれないが、多くの場合、それだけの価値がある。一方が説明し、一方が聞くことで、両者が学ぶことになるからだ。

　ロイド氏が今回のヒントを公開した理由は、コードレビューをやりとりする時間が最も負担になるからだという。何らかの理由でメンテナーが変更を好まないプロジェクトに投稿するときに、このようなことがよく起こるという。

　10のヒントを生かさないとどうなるか。最善の場合でも、無意味な議論に時間を浪費することになり、最悪の場合には、プロジェクトにおける貢献や多様性をぶち壊しにしてしまい、敵対的でエリート主義的な環境を作り出してしまうという。

　ロイド氏によれば、「明らかに……」「なぜあなたは……」といった言い回しを避けることはもちろん、コードレビューは客観的かつ簡潔であるべきだという。可能な限り確実性を扱うべきであり、政治的だったり、感情に流されたりする議論であってはならない。技術的な議論を心掛けなければならず、常にプロジェクトを前進させて、プロジェクトはもちろん、参加者をも高めることが目標だ。