以下はJacque Schragによるエントリ、10 Lessons Learned Conducting Code Reviewsの日本語訳です。
なお本文中唐突にHTMLとか出てくるので、Webアプリが前提の話のようです。
10 Lessons Learned Conducting Code Reviews
昨年秋、私の開発チームは一人(私だけ)から3人に増えました。
適切なプルリクとコードレビューは突然、非常に重要なものになりました。
大きく異なるバックグラウンドを持った複数の開発者が仕事に取り組んだ後で、一貫したコードベースが保たれていることが必要になります。
チームの開発主任として、私はコードレビューに責任を持つ立場になりました。
これまで一度もそれをしたことがなかったので、効果的にレビューを行うにあたって、私には学ぶべきことがたくさんありました。
以下に示すのは、過去数ヶ月のコードレビュー経験から学んだ10のヒントと教訓です。
1. Always Start with a Positive
常にポジティブで始める。
プルリクでは多くの指摘がなされます。
正しい要素が使われていない、モックアップと実装されたデザインが異なる、ロジックが間違っている、等々。
レビュアーとしてのあなたの仕事は、それらの間違いを探し、それらを修正するよう指摘することです。
そして、それらのネガティブ要素だけを抽出するのはとても簡単なことです。
しかし、意図的に間違いを犯す人は誰も居ません。
公にそのようなミスを指摘されることは、ネガティブで居心地の悪い経験になることでしょう。
全ての間違いを書き連ねることは、レビュアーにとっても楽しいことではありません。
ポジティブな雰囲気を出せるように、私はいつも感謝とうまくいった点からレビューを始めることにしました。
対処する必要があるコードに多くの問題点があったとしても、彼らの貢献に対して感謝の気持ちを表現することは大事です。
間違いのあるコードは、何もないコードよりは優れているので、問題を解決するための最初の段階を乗り越えたことに感謝するのです。
コードにレビューが付かないことは良いことである、と一般的には認識されています。
しかし、賞賛を形として表すことは有用なことです。
あなたがコードのチェックに時間を費やしたことがわかり、彼らは自信を持ち、フィードバックに積極的になります。
Xに対するロジックがよくなかったとしても、YやZに対してはうまくやってくれたと、彼らに知らせてあげてください。
2. It's not "you", it's "the code"
"You"ではなく"The"だ。
紛争を解決した経験があるのであれば、おそらくあなたは"I(私)"の重要性について学んでいるはずです。
"You did(あなたがやった)"ではなく"I think(私は考える)"、"I feel(私はこう思う)"のような表現を使用することによって緊張を和らげ、焦点を個人から問題にフォーカスさせます。
コードレビューについても同じ論理を適用しなければなりません。
指摘を行う際は、"You(あなた)"という言葉は避けなければなりません。
自分の作品に対する否定は受け入れがたいものです。
従って、間違っていると直接感じさせることは避けてください。
たとえば、
"Your logic on line #25 isn't clear.(あなたのコードの25行目がおかしい)"
ではなく、
"The logic on line #25 isn't clear to me.(このコードの25行目が、私にはちょっとわからない)"
とすることで、コメントのトーンは大きく変わります。
3. Use Line Item Suggestions
行単位コメントを活用する。
我々のチームはほとんどのプロジェクト管理にGitHubを利用しています。
私のお気に入り機能のひとつは、コードの特定行に対してコメントを投稿する機能です。
全体宛コメントでファイル名や行番号を書き出すことから生じる多くの面倒事や混乱を、事前に取り除いてくれます。
ひとつのコメントが単一の指摘であれば、これは修正する必要がある簡単なチェックリストを作成したのと同じことです。
参考：GitHubは行単位コメントの機能があります。@_darrenburnsの8 Productivity Tips for GitHubをご覧ください。
この機能を提供していない別のプラットフォームを使用している場合は、レビューに該当の行番号を含めるかリンクを含めることを強くお勧めします。
レビュー担当者としての労力は増えるかもしれませんが、必要な変更が見過ごされていないかを確認するのに役立ちます。
4. Set Project Standards Early & Tools to Help Review
プロジェクト標準を早期指定し、ツールに任せる。
我々はみな、コードを書く方法を別々の環境で学びます。
私が唯一の開発者であったときは、コーディングスタイル、命名規則、プロジェクト標準などについて心配する必要はありませんでした。
他者のコードレビューをし始めたとき、誰もが私と同じプロジェクト標準を持っているわけではなく、コードベースの混乱を招き、レビューに予想以上の精神的エネルギーが必要であることに気がつきました。
ここで学んだ教訓は単純です。
プロジェクト標準は早期に決定してください。
選んだものがどのような標準であれ、チーム全体がそれらを認識し、何が期待されているのかを知っていることが重要です。
使用する言語、命名規則、プロジェクトの構成、コメント規則など、全てが必要なものです。
余談：このプロセスの中で、私はCSSプロパティの順序について強いこだわりを持っていることに気がついたので、議論の中でそのことについて強調しました。
社内で開発した独自のプロジェクト標準を使用するか、多数存在する既存のプロジェクト標準のひとつを採用するかは、貴方方次第です。
しかし、プロジェクトの終わりに統一されたコードベースを作り上げたい場合は、その選択は極めて重要です。
Tools to Help Review
レビューツールを使う。
いちどプロジェクト標準を作成したら、その適用を支援するためのツールを設計することに時間を費やす価値はあります。
Linterはコードを分析し、構文について取り決めたガイドラインと比較することができます。
Linterはエラーを見つけたらフラグを立てますし、あるいは自動的に修正することも可能です。
コーディングスタイルに関するエラーをプルリク時にチェックするため、我々はTravisCIに追加のチェックを追加しました。
レビューを行うためにビルドしようとしてTravisが失敗したときは、原因を探るため深く調査を始める手間を減らしてくれます。
理想的には、ローカルでチェックして修正するとさらに良いでしょう。
5. Start Broad & then Dig Into Details
広く始めて細を穿つ。
ひとつのプルリクにはたくさんのコードが含まれる可能性があります。
コードレビューを実際に行うにあたり、大まかに始めて、徐々に核心に迫っていくのが私は好みです。
私の場合、そのプロセスは通常以下のようになります。
1.失敗したテストはありますか？そうであるならば、どうして失敗しましたか？
2.プルリクに含まれているべきではない、余計なファイルはありますか？
3.ブランチをエラー無しでチェックアウトできますか？
4.一緒にドキュメントは更新されましたか？
5.プロジェクトに新たなパッケージやdependenciesが追加されましたか？そうであるならば、何故追加されましたか？必要ですか？
6.HTMLはvalidですか？セマンティックですか？アクセス可能ですか？
7.CSSはプロジェクト標準に準拠していますか？ビューポートは考慮されていますか？ブラウザ間の互換性は保たれていますか？
8.JavaScriptはプロジェクト標準に準拠していますか？モジュール化されていますか？ロジックは整理されていますか？エッジケースは考慮されていますか？ログは出力されていますか？
最初の数ステップで問題が多く出てくるときはコードレビューをそこで終わらせ、より深いレビューをする前に、それらの問題が解決されるまで待ちます。
6. Establish Dedicated Review Time
レビューのための時間を確保する。
これは私にとって最も難しい教訓のひとつであり、いまだに苦戦しているものです。
徹底的で生産的なコードレビューを行うには、時間がかかります。
小さいプルリクであっても、大抵はブランチのチェックアウト、ロジックのチェック、問題のレビューなどが必要です。
優先する作業が他にもある場合は、そのプルリクを後回しにしたり、最低限のチェックしかせずに済ませたくなるかもしれません。
しかし、それは長期的には解決になりません。
これに対する私の解決策は、非常に単純なものでした。
コードレビュー専用の時間を設けたのです。
私はカレンダーにチェックを入れ、そして私のチームは、プルリクに対するレビューとフィードバックがいつ行われるかを知ることができます。
チームと自分自身に、予定を設定することが重要です。
7. Establish What You're Expecting
期待値をはっきりさせる。
プルリクをIssueにリンクしていますか？
プルリクに特定のフォーマットを指定していますか？
一回のプルリクに含めるコードの行数に制限はありますか？
質問のいずれかに対する答えがYesであれば、プルリクの期待値が何であるかを予め確立しておくことが重要です。
自分が求めているものが何なのかを開発チームが推測しなくても済むようにし、レビューを担当する時間を節約することができます。
時間をかけてチームメイトと話をし、全ての開発者にとって最適な方法を見つけてください。
8. Everyone Reviews
全員がレビュアー。
我々のチームでは、全員がコードレビューを行い、全員がコードレビューを受けます。
チームのシニア開発者として、私のプルリクは他2人のメンバーによってレビューされます。
なぜそうするかといえば、
・私のコードには間違いがあり、しばしば改善することができます。
・私が何故/どのようにそれを実装したのか、経験の浅い開発者に質問するチャンスを作ります。
良いコーディング方法を学ぶ最善の方法は、良いコードを読むことであると、私は信じています。
従って、これによって彼らは彼らの関わっているプロジェクトに関する良いコードを読むことができるようになるはずです。
我々は、コードレビューの中で、実装について時折話し合いました。
そして、私が彼らの質問に答えることができたなら、それは私の決断が正しいのだと自信を持つことに繋がりました。
9. Reevaluate Often
再評価を繰り返す。
コードレビューの実施は、私にとっても我々のチームにとっても大きな学習プロセスのひとつです。
我々はコードレビューの経験に基づいて、多くの変更を加えました。
プロジェクトのこの側面について、時間を割いて考えてみる価値はあります。
私は普段、プロジェクトの事後分析のひとつとして、それを行っています。
コードレビューを強化し、開発プロセスの重要な要素として織り込むのです。
チームメンバーに何がうまく行ったのか、何がうまく行かなかったのかを尋ね、そしてそれに正直に回答してもらいます。
どのフィードバックが有用であると感じたか、どの指摘はもっと改善することができるのか、それらを特定していきます。
レビューにどれくらいの時間を費やしたかを調べ、指摘された内容の傾向を探っていきます。
同じ種類の問題ばかりが頻繁にレビューで指摘されている場合、次のプロジェクトではその問題の発生が少なくできるように用意しておける手段はないでしょうか。
これらの問題を振り返って、チーム全員のレビュープロセスが改善されるような変更点を加えていきましょう。
10. There's more than One Solution
解決策はひとつではない。
これは難しいです。ほんとうに。
提出者のコードを、あなたであればそのコードをどのように実装するかということを反映するためにリファクタリングし始めることは、とても魅力的です。
セクション全体をあなたの"正しい"コードに書き直すことはとても簡単です。
しかし、それはコードレビューの目的ではありません。
たしかに、コードを丸ごと書き直さなければならないこともときにはありますが、しかし頻繁に起こるようなことではありません。
もしそのような目的なら、コードレビューより前にペアプログラミングを行うことがより良いアプローチになることでしょう。
しかし、問題に対するアプローチや解決方法が、あなたと同じ方法でないからといって、それが良くないということにはなりません。
プルリクを開いてレビューを開始するときは、まず自分のエゴを抑えつけ、新しいアプローチを受け入れるよう心を開いておかなければなりません。
自分がどれほど頭がいいのか披露するためにではなく、学ぶ機会としてレビューの時間を使ってください。
参考：この件については@maxwell_devのIt's Not About Youという素晴らしい記事があります。
これらの全てを学ぶ旅でした。
そして私がこれらのことを学ぶまでの間、継続的な忍耐力を発揮してくれたチームメイトの皆に感謝します。
コードレビューに接したことで、あなたが編み出したヒントはどのようなものですか？
コメント欄
「素晴らしい記事、ありがとう」
「最近ちょうどコードレビュアーのポジションになったので役に立った。#2が良い。」
「自分のチームはコードレビューでふたつのことを意識した。セキュリティ、パフォーマンス、判読できないコード、重複、クリーンコード、などなど、複数の問題があるときは、どれを優先するかを決める。これはとても役に立った。あとセキュアコーディング原則をカルチャーとしてチームに浸透させた。」
「前似たような記事書いたんで宣伝に来た。」
「#6は具体的にどれくらい取ったの？」「脳がまだ働いてる午前中に1～2時間」「ありがとう真似してみる！」
感想
参考になります(参考にするとは言ってない)。
レビュアーは単にコードだけ見ればいいというわけではありません。
そもそもプロジェクトの目的はコードを完成させることであり、コードレビューはあくまでその目的に対する手段のひとつに過ぎません。
そのためには、チーム全体のモチベを維持しながら開発能力を向上させていかなければなりません。
コードレビューひとつをとっても、硬過ぎないよう柔らか過ぎないよう繊細なハンドリングが必要なのです。