こちらの記事は、Elena Flat 氏により2020年4月に公開された『 How one code review rule turned my team into a dream team 』の和訳です。
本記事は原著者から許可を得た上で記事を公開しています。
本文
コードのレビューに費やす時間は、有意義な時間です。
次のシナリオを考えます。
プロダクトオーナーのボブは、3人の開発者（アリス、ケイス、デューク）のチームに、彼らが提出したばかりのコードに関して急速な修正のお願いをSlackで伝えています。 翌日の午前10時にクライアントとのミーティングがあり、ボブがデモできるように修正されていないといけません。 今は午後6時です。 アリスは退社していて、ケイスは別荘に行く途中です（彼女は休暇を取りました）。 しかし、デュークは偶然にも彼のノートパソコンの前にいます。
次に何が起こるか
- デュークはボブのメッセージを見て、問題なく変更できることをボブに知らせます。 しかし、彼は家族との約束があるので、明日の夜9時から朝までは出られません。 そのため、デュークは明日の午前中に修正反映のコミットのプッシュやサポートを行うことはできないかもしれません。
- 乗客として後部座席に座っているケイスがSlackでこれを見て、「問題ありません。何か仕上げが必要な場合は、午前中にそこにいるようにします」と言います。
- デュークはコードを書き始め、プルリクエスト（PR）をオープンします。
- 午後8時、アリスは家に着いてSlackを見て、今すぐ手助けできることをチームに知らせます。 彼女はデュークのPRを見て、急な対応だったためテストが抜けていることに気付き、テストを追加することにしました。 要件はSlackにあり、各自のフォークで共同作業できるため、これは問題ではありません。
- デュークは午後9時に業務から外れます。
- 午後10時、アリスはテストを終え、その頃にはケイスはオンラインになり、レビューができるようになっています。 ケイスはバグを見つけて（アリスはかなり眠いので、彼女は覚えていた要件のうちのいくつかを元に戻しました）、アリスはコードを少し変えた後、プッシュして反映させます。
- 朝になると、ケイスは、ボブからの土壇場での追加の要件のために、さらにいくつかの変更を加え、それらをプッシュして反映させます。 デュークはボブからの電話で伝えられた2つ目のPRを承認します（デュークは心配して出勤していました）。 クライアントとのミーティングは大成功です。
性別を逆にすると、これは実は私のチームで木曜日の深夜に起こったことです。 とても誇りに思いました。 珍しいことに、私が24歳のときに感じていた、スタートアップのために徹夜していたのは、ばかげたプライドのためではありませんでした。私を見てください。私の仕事は非常に重要です。もしも私が1日24時間働かなかったら、おしまいでしょう。
今回、私の誇りはさまざまなものによって支えられました。
- 私のチームの3人の開発者はすべて、コードに等しく精通しており、変更を加えて作業をレビューするために等しく準備できていました。
- チーム全体がこの機会に立ち上がって、1人の人を夜遅くまで仕事させるようなことはありませんでした。
- ３人全員揃っていたので、急いでも質の高い仕事ができました。
結果：
- それは緊急の作業でしたが、完全なテストカバレッジがありました。 したがって、技術的負債はありません。
- すべてのバグはレビューで発見されました。
- チームの友情とお互いに対する信頼は、史上最高でした！
同じチームで1年が経とうとしていますが、この頃になるとコードに慣れすぎてしまい、チームを変えた方がいいのでしょうか？ さて、検討すべき理由はいくつかあります。挑戦していますか？ ロードマップには何がありますか？ あなたのキャリアプランは何ですか？ ここではそのいずれにも触れません。 私が話したいのは、私が特にこのチームで働くことが大好きで、切り替えたくないということです。
それについて考えたとき、私は自分のチームで作業するのが好きだと気付きました。誰がチームにいて、チームで何をしているなどは関係なくです。
私は私たちのやり方が大好きです：
- チーム全体が私たちが所有するコードベースのほとんどを知っているので、すべての議論/設計＆アーキテクチャ構築を同時に行います。 これにより、非常に多様なアイデアと堅牢な技術設計が生まれます（幸い、私たちのチームはかなり多様性が高いです）。
- 一部の人が作業できない緊急時にそのメンバーの作業を分担することができます。
- 私たちは頻繁にリファクタリングと検証を行います。そしてそれを安全にできると思っています。
どうやってそこまで辿り着いたのでしょうか？ その秘訣はチームのために設定したプルリクエストレビューのルールとガイドラインです。
チームが結成されたとき、たった3人の開発者とプロダクトオーナー、マネージャーの半分しかいませんでした（私たちのマネージャーの残りの半分は、より大きなチームの管理で忙しすぎて参加できませんした）。 私たちは、私たちの人数が非常に少ないので、私たちは一丸になって仕事をした方が良いだろう と判断しました。 そこで私たちは、PRレビュールールを思い付きました。誰か一人が何を書いたとしても、残りの２人は冷静にレビューしないといけません。 それから、お互いのPRを一貫してレビューし続けるうちに、このルールセットをいくつかのガイドラインとしてまとめました。
私はこれらのルールとガイドラインをここに書き留めています。というのもこれらが自分たちのチームに役立つと感じる方もいるかもしれないと思ったからです。 あるいは、これらのルールは思考の糧を与えて、あなたは完全に異なるアイデアを思いつくことになるかもしれません。
ルール1
各PRレビューには、少なくとも同チーム内の開発者２人の承認がないとならない。 マネージャーの承認はカウントされません。
ここで最初にコメントすべきは、3人のチームとして開始したので、これは理想的だったということです。 3人の開発者全員が100％内部の事情に精通していました。 より大きなチームでは、これは異なる場合があります。 あなたが目指しているのは、ダンブルドアホークリュクスの状況です。あなたが死んでしまったとしても、少なくとも2〜3人がホークリュクスについて知っています。 私たちのチームは現在5人の開発者で、2人のベテラン、1人の新人、2人の平均的な開発者で構成されています。 私たちはまだ誰もがすべての人のPRをレビューするモデルに固執しようとしていますが、1人のベテランと他の1人の開発者の最小レビューユニットに切り替えることは意味があるかもしれません。
「マネージャーの承認はカウントされません。」 これを聞いてあなたは思うことでしょう。「どういうことだろうか？ 私はマネージャーであり、すばらしいコーダーであり、私のレビューは完全に重要のはずだ。よくもそんなことを。」と。 マネージャーのレビューは確かに役立ちます。 私たちには素晴らしいプログラマーである素晴らしいマネージャーがいます。彼らがレビューする時間があるとき、彼らはしばしば私たちを助けるための素晴らしいヒントやアイデアを持っています。
ただし、マネージャーがコーディングに費やす時間を考慮する必要があります。 それは50％以上でしょうか？ そうであれば開発者とみなせるので大丈夫。カウントしましょう。 ただし、当社では、開発マネージャーはほとんどコーディングはしていません。 結局のところ、記述されているコードと付き合っていかなければならないのは開発者です。 したがって、開発者は最終決定権を持ちます。 チームに3人の開発者がまだそれを見る機会がなかったとしても、マネージャーが1人の開発者のPRを承認し続けるという、マネージャーと開発者の無限の承認サイクルを見ることは私は本当に嫌いです。
ルール2
各PRには適切な説明が必要です。 説明を読むことで、レビュー担当者はコードが何をするのかを理解できるはずです。 Jiraチケットまたは要件ページがある場合でも、これは当てはまります。
説明のないPR —私のチームではこれは決して通過しません。 一番いいケースでは、Jiraチケットには適切な説明があるため、開発者が説明を書きませんでした。 最悪の場合、Jiraチケットには何も書かれていません。
説明のないPRを担当するレビュワーには2つの仕事があります。
- １つはPR中の変更された部分のコードを読み解いて、そのコードが何をしているか理解することです。
- コードが何をしようとしているのかを決定しようとしているときに、宇宙と現在の地政学的な気候についてのレビュー担当者の一般的な見解に基づいて決定を行おうとしていたらどうでしょうか？
コードの目的を明確に説明する記述がない場合、コードが正しいかどうかを確認することはできません。 何が正しいかわからないだけです。 レビュワーは自分が正しいと思うことを前提にレビューしてしまいます。
ルール3
PRには十分な単体テストと統合テストのカバレッジが必要です。
レビュー担当者が、通過したテストケースのリストを簡単に確認できる方法があることが理想的です。 テストカバレッジがすでに存在する場合、または何らかの理由でテストカバレッジが完了していない場合（タスクが別のチケットなどに分割されている場合）、PRの説明でこれについて言及されている必要があります。
優れたPRレビューを行うことは困難で時間がかかります。レビュワーはコードが何をすることになっているのかを理解したら、レビューを停止し、これに必要なテストカバレッジを書き留める必要があるためです。 独自のテストケースリストを作成し、その種類（単体テスト/統合テスト/エンドツーエンドテスト）を決定します。
また、レビュワーは運用に影響する可能性があるかどうか、およびデプロイ前に本番環境のデータを使ったテストが必要かどうかを検討する必要があります。
これですべてを理解できたので、レビュワーはPRのレビューに戻り、自分が考えたすべてのケースをカバーしているかどうかを確認します。 理想的には、ミックスになっていることです。ミックスというのは、レビュワーはPR申請者が考えなかったことを考えて、PR申請者はレビュワーが考えなかったことを考えたということです。それらが一緒になっていれば、レビュワーは良い仕事をします。 これは、PRでカバーされているテストケースのリストを簡単に確認できると便利です（対して、数百行または数千行のテスト行を読み取って、実際にテストされているケースを把握する必要があることを考えてみてください）。
ルール2が守られている場合は、説明を読むだけで、コードが何をするように書かれているかを判断できます。 したがって、実際のコードを確認する前に、テストカバレッジを確認する手順を実行できます。
私は個人的に、PRの説明に、これらすべて（テスト、本番環境データを使ったテスト、本番環境クライアントへの影響、本番環境構成の変更の必要性についての言及）をつけたいと思っています。この方法では全部が一つの場所にあります。 リンクされたJiraチケットがあるかもしれません。 それがどこかにあり、検討されてきたのである限りはそうでしょう。
ルール4
PRがバグ修正の場合、バグ修正が元に戻された場合、このテストは失敗するようなテストが含まれている必要があります。
これは言わなくてもわかることだと私は思っていましたが、大抵の明白なことでそうであるように、本当は明白ではありません。
あなたが==の代わりに！=にしてしまったタイプミスのバグがあるとしましょう。 午後10時だったので、あなたはとにかくコーディングするべきではなかったのですが、その夜はさらにもう少し進めることができる気がしました。 そして、あなたはバグ修正PRを開き、！=を==に変更します。 働いている親切な方がそれを承認し、その変更はマスターブランチに反映されます。
3日後、あなたの相棒のデュークは、制御不能になった彼のPRを最終的に準備しました。彼は、それが小さな機能だと思い、それを小さなチケットに分割しませんでした。しかし、それは事実ではなく、今彼はチームにレビューを求めているPRには2000行が存在します。
Dukeは上流からマージすると、！=を==に変更したところでコードが競合します。 デュークは疲れていて、何が起きているのか気づかず、！=を戻してしまいます。
それを捕らえるテストがないので、バグは戻ってきました。
これは人為的な例ですが、長期間にわたって注意を払うと、これは常に発生します。 3年以上かけてバグをいろいろな場所に配置してみてください。テストによってトラップされなかったバグが再現されるのを見ることになるでしょう。
以上！ 3つのルール、4つ目のルールは常識的な項目です。
これらのルールのおかげで私たちのチームはいくつかの非常に良質のコードを作成することができたと私は感じています。 私は非常に満足していますが、決してそのまま真似するようなことはしないでください。
繰り返しになりますが、これらのルールはあなたには正しい方法ではないと感じられるかもしれませんし、たまたま役立つ可能性もあります。 これらのルールが考えやアイデアを引き起こしたなら、私はそれで満足です！
翻訳協力
Original Author: Elena Flat
Thank you for letting us share your knowledge!
この記事は以下の方々のご協力により公開する事が出来ました。
改めて感謝致します。
選定担当: @r_pg10
翻訳担当: @r_pg10
監査担当: @_masa_u
公開担当: @r_pg10
私たちと一緒に記事を作りませんか？
私たちは、海外の良質な記事を複数の優秀なエンジニアの方の協力を経て、日本語に翻訳し記事を公開しています。
活動に共感していただける方、良質な記事を多くの方に広めることに興味のある方は、ぜひご連絡ください。
Mailでタイトルを「参加希望」としたうえでメッセージをいただく、もしくはTwitterでメッセージをいただければ、選考のちお手伝いして頂ける部分についてご紹介させていただく事が可能です。
※ 頂いたメッセージには必ずご返信させて頂きます。
ご意見・ご感想をお待ちしております
今回の記事は、いかがだったでしょうか？
・こうしたら良かった、もっとこうして欲しい、こうした方が良いのではないか
・こういったところが良かった
などなど、率直なご意見を募集しております。
いただいたお声は、今後の記事の質向上に役立たせていただきますので、お気軽にコメント欄にてご投稿ください。Twitterでもご意見を受け付けております。
みなさまのメッセージをお待ちしております。