株式会社エイチームライフデザインでエンジニアをしているasasigureです。
Ateam LifeDesign Advent Calendar 2024の12日目の記事になります。
私は最近レビュワーとして、メンバーのコードレビューを行うことが多いです。
レビューを重ねていくうちに、自分の中でのレビュー依頼の良し悪しを考えるようになりました。
自分なりに、レビュー依頼をするときに意識すること、してほしいことをメンバーへ伝えるために言語化する機会があり、せっかくなのでアドカレの記事として一本書こうと思い立ちました。
対象読者
- 1,2年目のエンジニア
- レビュー依頼で大事なことがイマイチわからない方
ゴール
目指すゴールは、「レビュワーにやさしいレビュー依頼ができるようになる」ことです。
レビュワーにやさしいレビューとは何か、なぜそれが必要なのか理解したうえで、具体的な行動についても示していきます。ぜひ参考にしてください。
※記載する内容は私個人の意見です。合わない意見もあるかと思いますが上手に取捨選択していただけると助かります。
やさしくないレビュー依頼
やさしいレビュー依頼を考えるには、まずやさしくないレビューがどんなものなのか知っておくとイメージがしやすくなります。どんなレビュー依頼がレビュワーにとって優しくないのでしょうか。
目的や仕様についての記載が不足している
目的や仕様についての記載が不足していると、レビュワーが依頼者(実装者)へ目的を確認をする手間が発生したり、関連するコードをすべて読んで実際に環境を立ち上げて仕様を確認/推測しなければならず、大きな手間がかかります。
記載されていたとしても、情報がまとまった場所になければやはり手間がかかります。例えば、仕様についての議論をIssueのコメントで行い、仕様が固まったとしましょう。その内容をIssueのコメントに記載があるからといって、まとめずにそのままの状態でレビュー依頼をしたらどうなるでしょうか。レビュワーはIssueのコメントのやりとりを追っていけば仕様を理解することができます。しかし、レビューをするうえで不必要な情報まで目を通すことになります。目的や仕様について、決まったものは情報としてまとめておくべきでしょう。
検証内容の記載がない
検証が不足している状態は、コードが正しく動作することを保証できていないということです。経験豊富なレビュワーであれば、検証内容が記載されていなくとも、変更されたコードを見て正しく動作するか判別できるかもしれません。しかし多くの場合は、関連するコードを確認したり、実際に環境を立ち上げて確認することになり、大きな手間になってしまいます。
検証内容の記載がない場合、そのコードが仕様どおりに動作しているのかわかりません。経験豊富なレビュワーであれば、検証内容が記載されていなくとも、変更されたコードを見て正しく動作するか判別できるかもしれません。しかし多くの場合は、関連するコードを確認したり、実際に環境を立ち上げて確認することになり、大きな手間になってしまいます。
検証内容が記載されていれば、コードが仕様どおりに動作していることがある程度は保証されます。そして全く検証がなくゼロから指摘をするよりも、検証内容が不足していて指摘する方が具体的な内容を伝えることができます。不足していた原因を探って、依頼者(実装者)へ知識や経験を伝えることもできます。これは依頼者(実装者)の知見を深めることにもなりますし、有意義です。
実装方針や、相談したい実装部分などが記載されていない
実装方針や相談したい実装部分が記載されていない場合、レビュワーは依頼者(実装者)がどのように考えてコードの変更をしたかわかりません。内容や粒度によっては記載がなくてもとくに問題ないものもあります。しかし、レビュワーから見て問題のあるコードの変更があった場合、依頼者(実装者)が悩んだ結果その実装になってしまったのか、問題だと認識しないまま実装したのかがわかりません。レビュワーが依頼者(実装者)へ確認をする必要があります。
依頼者(実装者)の知見を深めるためにも、実装方針や相談したい部分を記載することは大事です。
普段からちゃんと記載をしていれば、もし問題のあるコード変更でコメントがなかった場合、レビュワーはその部分について依頼者(実装者)が問題を把握できていないことがわかります。問題を把握できていない部分がわかれば、理解が浅いポイントに絞って依頼者(実装者)へ知識や経験を伝えることができます。
通常のリリース手順ではない場合に手順の記載がない
通常のリリース手順ではない場合とは、例えば「A をリリースする前後に〇〇をする」「A をリリースする前に B をリリースする」などです。
リリース手順の記載がなく、リリース時にも気が付かずに通常の手順どおりのリリース対応をした場合、リリースに失敗したり、最悪の場合大きな不具合を招くことになります。
やさしいレビュー依頼をするための心構え
ここまではやさしくないレビュー依頼の話をしました。やさしいレビュー依頼は単純な話、やさしくないレビュー依頼の逆のことをすればよいのです。ただしそれだけだと方法論の話になってしまい、形骸化してしまいます。
やさしいレビュー依頼をするための行動について話す前に、行動をするために、どんなことを意識すべきかを考えて見ましょう。
レビュアーのレビューコストをできる限り下げようとする
レビュアーはおそらく依頼者(実装者)より上位の役職であったり、多くの業務に追われている人が多いのではないでしょうか。やさしくないレビュー依頼はレビュアーの時間を無駄に奪ってしまうことになります。レビューコストを下げるためにできることを考えましょう
※レビュー依頼の中身の話で、レビュー依頼の数を減らすのはよくありません
レビューは責任の分譲
レビュー前までは、コード変更についての責任はすべて依頼者(実装者)にあると言えます。しかし、レビュアーがレビューOKを出した後でも責任はすべて依頼者(実装者)にあるといえるでしょうか？私は双方に責任があると思います。
レビューは、依頼者(実装者)が実装物についての責任をレビュアーへ分譲する行為と捉えられるでしょう。
責任の分譲は決して軽々しい行為ではありません。そんな行為を行うのですから、依頼者(実装者)はレビュー依頼を丁寧に、わかりやすいものにすべきです。すなわち、やさしいレビュー依頼をしようということです。
レビュアーは全知全能ではない
レビュアーはコードすべてを把握しているわけではありません。不備や不足を必ず見つけることができるわけではありません。優しくないレビュー依頼であればあるほど、抜けや漏れが発生しやすくなります。(私自身のことで言えば、みんなが思っているよりしっかりしてないし、なんでも把握しているわけではない、正直今でも自信がない！！)
やさしいレビュー依頼
意識すべきことも話しましたので、やさしいレビュー依頼をするための行動について話しましょう。先程も述べた通り、基本的には優しくないレビュー依頼の逆のことを行えばよいのです。
目的や仕様について記載する
目的や仕様について記載をしましょう。目的や仕様があって初めてコード変更をする必要性が生まれます。そのため基本的にはコード変更前に目的と仕様は記載しましょう。
恐ろしいことに、目的が曖昧な状態で、何故かコード変更だけが進むケースはよくあります。本来の目的だったはずのものを見失い、意味のないコード変更に時間を使ってしまうことになります。やはり目的はコード変更前に記載しましょう。
情報を集約する
目的や仕様はもちろんのこと、その他の情報が一箇所にまとまっている状態にしましょう。そして、最新の状態を維持しましょう。レビュー中に情報を更新する場合は、レビュワーに変更した旨を伝えるのと良いでしょう。
検証内容を記載する
開発環境での検証内容はもちろんのこと、本番やステージング環境での検証項目も記載しておきましょう。リリース時の検証時に役立ちます。
検証結果の内容は、before/afterのスクリーンショットなどを記載すると、変更内容がわかりやすくレビュワーに伝わるでしょう。
特にみてほしい、相談したい実装部分を記載する
みてほしい、相談したい部分において、「こう考えたので、この手段をとりました/とりたい」を記載しましょう。悩んだこともあればそれも記載すると良いでしょう。
コード変更が複雑になっている場合は、補足で説明を記載することも考えてください。
通常のリリース手順ではない場合に手順を記載する
その手順を上からやっていけば問題なくリリースができるように手順を記載しましょう。対応時の漏れやミスを防ぐことができます。
おわりに
ここまでの内容を読んで、すぐにレビュワーにやさしいレビュー依頼ができるようになることは難しいと思う方もいるかも知れませんが、慣れるまで辛抱強く意識してレビュー依頼をしてみましょう。