私は最近、コードレビューのベストプラクティスに関する記事を読んでおり、これらの記事がバグの発見に焦点を当てており、レビューの他のコンポーネントをほとんど無視していることに気付きました。 特定された問題の建設的かつ専門的な議論ですか？ 関係ありません！ すべてのバグを見つけるだけで、それが追加されます。

だから、私は啓示を受けました：これがコードで機能するのなら、なぜロマンチックな関係で機能しないのですか？ そのため、プログラマーと恋人との関係を支援する新しい電子書籍をご覧ください（左の図をご覧ください）。

私の革新的な本は、パートナーの欠陥の最大数を特定するための実証済みのテクニックを教えます。 本は 、次の領域をカバーしていません 。

•共感と理解を伴う問題の議論。
•欠陥を修正するためのパートナーの支援。

コードレビューに関する文献を読んで理解できる限り、関係のこれらの部分は非常に明白であるため、議論はまったくありません 。

そのような本はどうですか？ あなたは本当に好きではないと思います。

では、なぜこのようにコードレビューを行うのですか？

私が読んだ記事は、すべての開発者がロボットである未来からのものであると仮定することができます。 この世界では、あなたの同僚はコードの不注意な批判を歓迎します。そのような情報の処理は冷たく心を温めるからです。

ロボットではなく人と作業する現在のコードレビューを改善したいという大胆な前提を立てます。 同僚との良好な関係が主な目標であり、エラー修正を高速化するための変数（欠陥あたりのコストパラメーターを最小化する）だけでなく、さらに大胆な仮定です。 これらの状況に照らして、レビュー方法はどのように変わりますか？

この記事では、コードレビューが技術的なだけでなく、社会的なプロセスであることを示唆する手法について説明します。

コードレビューとは何ですか？

「コードレビュー」という用語は、開発者の背後からコードを読むことから20人の会議まで、さまざまなアクションを意味し、そこでコードを1行ずつ解析します。 ここでは、正式な文書化された手順に言及していますが、コード検査のための一連の会議に負担はかかりません。


レビューコードには、コードを作成してレビューに送信する作成者と、コードを分析してプロジェクトの一般的なコードベースに追加する準備ができたときに決定を行うレビュー担当者が参加します。 何人かのレビューアがプロセスに参加するかもしれませんが、簡単にするために、それが1人であると仮定します。

レビューを開始する前に、作成者は変更のリストを作成する必要があります。ここで、共通のコードベースに加えたいすべての変更をリストします。

レビューは、作成者がレビュー担当者に変更のリストを送信すると開始されます。 それは数ラウンドで起こります。 各ラウンドは、作成者とレビュアーの間の1回の完全な往復です。作成者は変更を送信し、レビュアーはこれらの変更に対する応答を返します。 各コードレビューは、1つ以上のラウンドで構成されています。

レビューアが変更を承認すると、レビューは終了します。 これには通常、LGTMメッセージの送信が伴います。これは、「私には良さそうです」という短縮フレーズです。

なぜそんなに難しいのですか？

プログラマーが驚くと思う変更のリストを送信し、彼が正しくない理由の詳細なリストで彼に答える場合、そのようなコミュニケーションには特定の繊細さが必要です。

「プログラマが非常に魅力のない人々であるため、これが私がITを見逃さない理由の1つです。たとえば、航空機では、能力を過大評価している人々はすでに死んでいます。」

Founders at Workの ArsDigitaの共同設立者であるPhilip Greenspan氏。

著者は、自分が無能なプログラマーであるという意味で自分のコードを批判することは非常に簡単です。 コードレビューとは、知識を共有し、エンジニアリングに関する有意義な決定を下す能力です。 しかし、これは著者が議論を自分に対する個人的な攻撃と見なす場合は不可能です。

これがないと十分な困難はないかのように、ここであなたは自分の考えを書面で表現する必要があり、それはさらに誤解のリスクを高めます。 作者はあなたの声の調子を聞かないし、ボディーランゲージを知覚できないので、言葉遣いを注意深く選択することはとても重要です。 守備的な立場にある著者にとって、「ファイル記述子を閉じるのを忘れた」などの無害なメモは、「ここでファイル記述子を閉じるのを忘れたとは思わない！」 あなたはそのようなばかです！」

技術者

1. コンピューターに退屈な仕事を与える
2. スタイルガイドにスタイル引数を入力します
3. すぐにレビューを開始
4. 高レベルで開始し、下に行く
5. コードサンプルをgeneしみなく使用する
6. 決して言わない
7. チームではなくリクエストとしてレビューを行う
8. 意見ではなく原則で正当化する

コンピューターに退屈な仕事を与える

ミーティングやメールに常に気を取られている場合、思慮深いコード分析の時間を作るのは困難です。 精神力は時間よりもさらに小さいです。 同僚のコードを読むことは認知的に困難な作業であり、高度な集中力が必要です。 特にパフォーマンスが良い場合は、コンピューターが実行できるタスクにこれらのリソースを浪費しないでください。

明らかな例は、スペースの字下げによるエラーです。 自動フォーマットツールを使用した場合と比較して、人からのそのようなエラーを特定するのにどれだけの労力がかかるかを比較してください。

手動編集に必要なもの	自動フォーマットツールを使用する場合に必要なもの
余分なスペースと間違ったインデントを見つけます。 コードを正しくフォーマットするためのコメント呼び出しを記述します。 発言を読み直し、告発のヒントなしに、明確に定式化されていることを確認してください。 著者はコメントを読む必要があります。 作成者はコードのインデントを修正します。 レビューアは、作成者がすべてを正しく修正したことを確認します。	。 。 。 。 。 。 。 何もない！

作成者は、[保存]ボタンをクリックするたびにスペースを自動的にフォーマットするコードエディターを使用したため、右側は空です。 まあ、最悪の部分は、作成者が検証のためにコードを送信すると、継続的統合システムが誤ったスペースを報告することです。 作成者は、レビュー担当者が気付く前に問題を修正します。

自動化できる他の機械的アクションを見てください。最も一般的なものは次のとおりです。

挑戦する	自動化ソリューション
ビルドを確認する	TravisやCircleCIなどの継続的統合システム。
自動テストを確認する	TravisやCircleCIなどの継続的統合システム。
インデントとスペースが企業スタイルであることを確認します。	ClangFormat （C / C ++の場合）またはgofmt （Goの場合）などのコードフォーマットツール。
未使用モジュールまたは未使用変数の識別	pyflakes （Pythonのlinter ）やJSLint （JavaScriptのlinter ）などの静的コードアナライザー。

オートメーションは、レビュアーとして、より価値のある貢献をするのに役立ちます。 モジュールのインポート順序やソースファイルの命名規則などの問題のクラス全体を無視すると、機能エラーやコードの読みやすさの低下など、より興味深いことに集中できます。

自動化は著者にとって有益です。 これにより、数時間ではなく数秒で不注意なエラーを検出できます。 作成者の関連コンテキストはまだ作業メモリにあるため、インスタントフィードバックにより学習が高速化され、エラー修正が簡素化されます。 さらに、作者はあなたからではなく、コンピュータからの愚かなエラーに関するメッセージをはるかに簡単に知覚します。

コードレビューワークフローでこの種の自動チェックを実装するために協力する必要があります（たとえば、Gitでのコミット前のフックやGitHubでのwebhook ）。 レビュープロセスで作成者がこのようなチェックを手動で実行する必要がある場合、ほとんどの利点が失われます。 作成者は検証を忘れることが避けられないため、自動検証で修正できる単純なエラーに煩わされる必要があります。

スタイルガイドにスタイル引数を入力します

スタイルディスカッションは、コードレビュープロセスの時間の無駄です。 もちろん、統一されたスタイルが重要ですが、コードを議論することは、ブレースをどこに置くかを議論するのにふさわしい時ではありません。 プロセス全体から論争とスタイルを排除する最善の方法は、スタイルガイドを入手することです。



優れたスタイルガイドは、命名規則やインデント規則などの外部スタイリング要素だけでなく、特定のプログラミング言語の機能の使用方法も定義します。 たとえば、JavaScriptとPerlには機能が満載されています-同じロジックを実装するための多くのオプションがあります。 スタイルガイドでは1つの正しいプログラミング方法が定義されているため、チームの半分が1つの言語関数セットを使用し、残りの半分が完全に異なる関数セットを使用することはなくなります。

スタイルガイドがあればすぐに、ファイルに名前を付ける方が良いという論争で、著者とメッセージを交換するためにレビューサイクルを費やす必要がなくなります。 ガイドに従ってください。 マニュアルに特定の問題に関する指示が含まれていない場合、通常は議論するべきではありません。 マニュアルに記載されていないが議論に値するスタイルについて質問がある場合は、チームと一緒に解決してください。 次に、この議論に二度と戻らないように、スタイルガイドに決定を書きます。

オプション1：既存のスタイルガイドの調整
インターネットを検索すると、公開されているスタイルガイドを見つけて、自分で使用するために借りることができます。 最もよく知られているのはGoogleスタイルガイドですが、これらが機能しない場合は他のガイドを見つけることができます。 既存のドキュメントを適合させることにより、スタイルガイドを作成することで、そのようなドキュメントを最初から作成する時間を無駄にすることなく、すべての利点を得ることができます。

欠点は、すべての組織がこれらのルールを内部のニーズに適合させることです。 たとえば、Googleのマニュアルは、新しい言語機能の使用に関しては非常に保守的です 。なぜなら、ホームルーターから最新のiPhoneまで、すべてに機能するはずの巨大なコードベースがあるからです。 単一の製品で4人のスタートアップを抱えている場合、最新の機能や言語拡張機能を使用する際により積極的な戦略を選択するのが理にかなっています。

オプション2：独自のスタイルガイドを徐々に完成させる
既存のドキュメントを変更したくない場合は、独自のドキュメントを作成できます。 コードのレビュー中にスタイルの議論が発生するたびに、正式な合意がどうあるべきかをチーム全体の前に質問します。 合意に達したら、この決定をスタイルガイドに入れます。

バージョン管理システム（たとえば、 GitHub Pages ）でマニュアルをMarkdown形式で保持することを好みます。 そのため、すべての変更は標準のレビュー手順を経ます。誰かが明示的に変更を承認する必要があり、チームの全員が問題を提起できます。 WikiおよびGoogleドキュメントも適しています。

オプション3：ハイブリッドアプローチ
オプション1と2を組み合わせて、既存のマニュアルを適合させると同時に、ローカルドキュメントを維持して、データベースを拡張または書き換えることができます。 良い例は、Chromium C ++のマニュアルです。 そこで、彼らはGoogleのC ++マニュアルをベースとして、独自の変更と追加を行いました。

すぐにレビューを開始

コードレビューを優先度の高いタスクとして検討してください。 コードを勉強するとき、またはレビューを書くときは、時間をかけますが、すぐにそれを始めましょう-理想的には数分以内に。



同僚が変更のリストを送信した場合、フィードバックを受信するまで彼の作業が中断された可能性があります。 理論的には、バージョン管理システムにより、別のブランチを作成して作業を続行し、新しいブランチでレビューの変更を投稿できます。 実際には、これを効果的に行うことができる開発者は世界中に約4人います。 他のすべての人にとって、三国間の差を解くには非常に時間がかかり、レビューを見越して行うことができる進歩を中和します。

すぐにレビューを開始する場合、好ましいサイクルを作成します。 ワークフロー全体は、作成者の変更リストのサイズと複雑さの関数のみになります。 これにより、著者は、小さく正確な言葉のリストを送信することができます。 レビューはより簡単で快適に検討できるため、レビューが加速され、好循環が続きます。

同僚が1000行のコードを変更する必要がある新しい関数を実装したと想像してください。 約2時間で200行の変更リストを確認できることを彼が知っている場合、彼はこの機能を約200行のフラグメントに分割し、1〜2日で機能全体の確認を終了します。 ただし、サイズに関係なく、コードのレビューに1日かかる場合、関数の承認には1週間かかります。 同僚は1週間待ちたくないので、それぞれ500〜600行の大きなリストを送信します。 200行よりも600行の変更に対してコンテキストを維持するのが難しいため、検討するのが難しく、レビューが悪くなります。

1回のレビュー期間の絶対最大値は1営業日である必要があります。 優先度の高い問題で忙しく、1営業日に収まらない場合は、同僚にそのことを知らせ、レビューのために他の人を選択する機会を与えます。 1か月に1回以上レビューを中止しなければならない場合、適切な開発プラクティスを維持するために、チームがレースのペースを下げる必要がある可能性があります。

高レベルで開始し、下に行く

特定の各ラウンドで書くメモが多いほど、作者が落ち込んでいると感じるリスクが大きくなります。 正確な制限は開発者によって異なりますが、危険ゾーンは通常、1ラウンドのレビューあたり20〜50音で始まります。

コメントの海に著者をauthorれさせたくない場合は、最初のラウンドで高レベルの編集のみに制限してください。 クラスインターフェイスの再設計や複雑な機能の分割などの問題に集中します。 これらの問題が修正されるのを待ってから、変数の命名やコード内のコメントのわかりやすさなどの低レベルの問題に移ります。

作成者が高レベルの編集を行うと、低レベルのメモは無関係になる可能性があります。 それらを後のラウンドに移動すると、これらの物議を醸すトピックに関するコメントの単語を慎重に選択するという重要な作業から身を守ることができ、作成者はオプションのコメントを処理する必要がなくなります。 また、この手法は、レビュープロセスに集中する抽象化のレベルをセグメント化し、あなたと著者が変更のリストを明確かつ体系的に確認できるようにします。

コードサンプルをgeneしみなく使用する

理想的な世界では、コードの作成者はレビューに感謝します。 彼にとって、これは学ぶ機会であり、間違いからの保護です。 実際には、著者がレビューを否定的に認識し、コードに対してコメントをしていることにinすることができるため、多くの外部要因があります。 締め切りのせいで急いでいるのかもしれないので、即座の承認を除いて、どんなヒッチも障害のように思えます。 たぶん、あなたはそれほど長く一緒に仕事をしておらず、彼らはあなたの発言が善意で作られているとは信じていません。

レビュー作成者によるコードの認識を改善する良い方法は、彼にプレゼントを贈る機会を見つけることです。 そして、すべての開発者はどのような贈り物を受け取るのが好きですか？ もちろん、コード例。



あなたが行うことを提案するいくつかの変更を書くことによって著者の仕事を促進するならば、あなたはあなたがレビュアーとして時間をgeneしみなく共有することを示します。

作成者がPythonのリスト関数にあまり精通していないとします。 彼は、レビューのために次の行を含むコードを送信します。

urls = [] for path in paths: url = 'https://' url += domain url += path urls.append(url) 

「これはリストを含めることで単純化できますか？」というスタイルの答えは、これまでに使用したことのない関数を20分かけて学習する必要があるため、迷惑です。

著者は、そのようなメモをとることがはるかに幸せです：

この種のリストを含めることを検討することを提案します。

 urls = ['https://' + domain + path for path in paths] 

この手法は、単一行の変更に限定されません。 多くの場合、独自のコードブランチを作成して、大きな関数を分割したり、追加の境界状況をカバーする単体テストを追加するなど、優れた概念的なアイデアを作成者に示します。

この手法は、明確で否定できない改善のためにのみ使用してください。 上記のリストに含まれる例では、コードの行数が83％削減されると主張する開発者はほとんどいません。 逆に、個人の好みに合った「より良い」変更（たとえば、スタイルの変更）を示すために長い例を書くと、このサンプルコードは、寛大さを示すのではなく、設定を永続的にプッシュしているような印象を与えます。

ラウンドごとに2つまたは3つのコード例に制限してください。 著者の変更リスト全体のコードを書き始めると、著者自身がこのコードを書くことができるとは思わない印象を与えます。

決して言わない

少し奇妙に聞こえますが、耳を傾けてください。コードレビュープロセスで個人的に著者に連絡しないでください。

あなたの決定は、誰がこれらのアイデアを思いついたのではなく、コードを改善するためのアイデアに基づいているべきです。 あなたの同僚は彼の変更リストとコードに多大な努力を費やしており、おそらくその仕事に誇りを持っています。 彼の作品に対する批判に対する自然な反応は、守備的な立場になることです。

防御的な姿勢のリスクを最小限に抑えるために、レビューの言葉を選択してください。 開発者自身ではなく、コードを批判していることを明確にしてください。 作者が解説で彼に個人的な魅力を見ると、これはコードから注意をそらし、その人に注意を向けます。 そのため、彼は個人的に批判を受けるリスクが高まります。

たとえば、無害なコメントを次に示します。

「安全に」という言葉を間違えました。

著者は、このような発言を2つのまったく異なる方法で解釈できます。

• 解釈1 ：ちょっとバディ！ 「安全に」という単語をつづりました。 しかし、あなたはまだ賢いと思います！ これはおそらく単なるタイプミスです。
• 解釈2 ：「安全に」という言葉を間違えました、バカ。

これをあなたが対処されていない発言と比較してください。

成功->成功

2番目の注記は単なる修正であり、著者による評価ではありません。

幸いなことに、「あなた」という言葉を避けることで、コメントを簡単に書き換えることができます。

オプション1：「あなた」を「私たち」に置き換えます

この変数に、 seconds_remainingような、よりseconds_remaining名前を付けることができますか？

になります：

この変数に、 seconds_remainingようなよりわかりやすい名前を付けることができますか？

「私たち」は、コードに対するチーム全体の共同責任を強化します。 著者はあなたと同じように別の会社に行くことができますが、何らかの形でこのコードを担当するチームが存在します。 著者が単独で明らかにしなければならない変更に関しては、「私たち」と言うのは馬鹿げているように思えるかもしれませんが、告発者よりも愚かに見える方が良いです。



オプション2：文から件名を削除する
個人的な扱いを避けるための別のオプションは、文から主題を除外する略語を使用することです：

seconds_remainingような、よりseconds_remaining名前に変数の名前を変更することを検討する必要があります。

同じ効果は、受動音声を使用しても実現できます。 私は通常、ペストのような技術文書では受動態を避けますが、これは個人的な取り扱いの問題を回避するための便利な方法です。

この変数は 、 seconds_remainingような、よりseconds_remaining名前に変更する必要があります 。

別のオプションは、「What about ...」または「How about ...」という単語で始まる質問の形式で文を言い換えることです。

seconds_remainingような、よりseconds_remaining名前にこの変数の名前を変更するのはどうですか。

チームではなくリクエストとしてレビューを行う

レビューコードは、議論が個人的な議論に滑り込むリスクが高いため、通常のコミュニケーションよりも多くのタクトと注意が必要です。 レビュー担当者は、個人的なコミュニケーションと比較して、レビューコードをより丁寧かつ丁寧にすべきであると思われます。 しかし、私は奇妙な方法で正反対の状況を発見しました。 多くの人が同僚に「このホッチキスをくれて、ソーダを持ってきてくれ」とは決して言わないでしょう。 しかし、「このクラスを別のファイルに転送する」など、レビュアーがこのようなチームスタイルでレビューを発行するケースを多く見ました。

しかし、あなたのコメントで迷惑なほど礼儀正しくしないでください。 チームとしてではなく、リクエストまたは提案として作成してください。

同じコメントを2つの異なる方法で比較します。

コマンドとして発行されたメモ	リクエストとして発行された発言
Fooクラスを別のファイルに転送します。	Fooクラスを別のファイルに転送できますか？

人々は自分の仕事を管理したい。 そのような要求は、著者に独立感を与えます。

このような要求は、著者が丁寧に対応する可能性も高めます。 たぶん、彼はそのような選択をする彼自身の理由を持っています。 コマンドの形で発言する場合、著者の意見の不一致は不服従の形をとります。 リクエストまたは質問としてコメントを作成した場合、作成者は単に答えることができます。

最初の発言のデザインに応じて、会話の好戦的な様子を比較します。

チームとして発行されたフィードバック（攻撃）	リクエストとして発行されたフィードバック（協力）
レビュアー ： Fooクラスを別のファイルに転送します。 作成者 ：これは、 Barクラスから離れすぎてしまうため、行いたくありません。 顧客はほとんどの場合それらを一緒に使用します。	レビュアー ： Fooクラスを別のファイルに転送できますか？ 作成者 ：これはできますが、それではBarクラスから離れすぎてしまい、クライアントは通常これら2つのクラスを一緒に使用します。 どう思いますか？

あなたの論文を証明するために架空の対話を構築したとき、文明化されたコミュニケーションがどれほど多くなったのか、コマンドではなくリクエストの形で発言しましたか？

意見ではなく原則で正当化する

著者にコメントするときは、提案された変更とその理由の両方を説明してください。 「このクラスを2つに分割する必要があります」というフレーズの代わりに、「このクラスがファイルのダウンロードと解析の両方を担当するようになった」と言う方が良いでしょう。 これは、共有責任の原則に従って、ダウンロード用のクラスと解析用のクラスに分割する必要があります。

原則で発言を正当化する場合、議論は建設的な形を取ります。 「クラスのオープンインターフェースを最小化するには、この関数をプライベートにする必要がある」などの特定の理由を挙げた場合、著者は単に「いいえ、自分のやり方でやりたい」と答えることはできません。 そして、もし彼がそれに答えるなら、それは馬鹿げたものに見えるでしょう。なぜなら、変化がどのように目標を達成することを可能にするかを示したからです。

ソフトウェア開発は芸術でもあり科学でもあります。 確立された原則の観点から、特定のコードフラグメントの何が間違っているのかを正確に定式化することは常に可能とは限りません。 コードが単純にい、または過度に複雑な場合があり、正確な理由を明確にすることは困難です。 そのような場合は、できる限り説明しますが、客観性を保ちます。 「 これは理解するのが難しいように思えます」と言うと、これは少なくとも客観的な声明であり、「 これは混乱を招くコードです」というフレーズとは対照的です。

可能な場合は、リンクの形で裏付けとなる証拠を提供してください。 チームのスタイルガイドの適切なセクションは、提供できる最良のリンクです。 言語またはライブラリのドキュメントも参照できます。 高い評価のStackOverflow回答も適していますが、信頼できるドキュメントから離れるほど、証拠は不安定になります。

パート2：近日公開

数週間のうちに、記事の第2部を公開します。 連絡を取り合い、次のような追加のヒントを検討します。

• 大きすぎるコードレビューで作業します。
• 高い評価を与える能力を認識する。
• レビューフレームワークへの準拠。
• 絶望的な状況での行動。

UPD。 記事の2番目の部分が公開されています。