こんにちは。駅奪取エンジニアの
id:dorapon2000です。
コード差分の大きなプルリクエスト（以下、プルリク）をコードレビューした経験は多くの方があると思います。 プルリクは小さく・単位ごとに、とは頭でわかっていても、実装している内に想定よりも大きくなってしまったり、1つのプルリクにまとめなければコンテキストが伝わらなかったり、どうしてもということはあります。本当に申し訳ないと思いつつレビュー依頼を出したり、出されたり。
今回は、巨大なプルリクを前にして、自分がどうモチベーションを保つか、どう読み解いていくか、同じ状況を避けるためにレビューイと協力できることはなにか、について自分の場合を例に紹介していきます。
プルリクは小さく
ここでは巨大なプルリクは避けるべきだという前提で記事を進めていきます。 つまり、プルリクは可能な範囲で小さい方がよいという前提です。 その理由は、コードレビューに関する優良な記事が多くあるため、そちらに譲ります。
また、どれほどで巨大かは主観で大丈夫です。レビュアーの心のソウルジェムが濁り始めたら、この記事が役に立つかもしれません。
リスペクトを忘れない
自分はレビューイに対するリスペクトがコードレビューで一番大切だと考えています。それは巨大なプルリクでも同様です。 悪意があってプルリクを大きくしたわけではありませんし、実装も大変だったはずです。 よりよいコードへするために協力するという気持ちとねぎらいを忘れないようにしています。
読み解く3つの選択肢
巨大なプルリクには様々なコンテキストが埋め込まれており、レビュアーはApproveする前にそれらを理解する必要があります。 理解にかかる時間と体力が、頭を重くする理由です。
モチベーションを保つためにも、理解しやすくするためにも、自分は3つの選択肢を考えます。
- プルリクを分割してもらう
- 会話をしながらレビューイに説明してもらう
- 簡単なもの⇒メイン処理の順に読む
1. プルリクを分割してもらう
プルリクが大きいならば、分割することで1つあたりの負担も小さくなります。
まず一番最初に考える選択肢ですが、同じことはレビューイも考えており、たいてい分割できません。 現状から分割するには余計に実装コストがかかったり、そもそも分割するほうがレビューが大変になる場合です。
2. 会話をしながらレビューイに説明してもらう
弊社は完全リモートなため、通話をしつつ画面共有してもらいながら、プルリクの各差分の意味を説明してもらいます。
会話のメリットは、なにより心理的な負担が低いことです。自分でコードを追わずとも、何をする箇所に、どういう変更を、どういった意図で入ったのかを理解できます。 プルリクが巨大なため、疑問点も多く出てくるでしょう。それも通話であれば即座に解決します。このスピード感は非同期でするレビューにはない魅力です。
もちろん、通話している分だけレビューイの時間も必要です。しかし、通話のあるなしで両者がレビューに掛ける合計時間の差はないように感じます。
会話へ入る前に、レビューイは事前解説コメントをプルリクエストに残したり、レビュアーはプルリク全体を眺めておくことで、よりスムーズな会話になります。 また会話後も、会話の内容をプルリクエスト内に記載しておくことで、実装意図を見返す際や他のチームメンバーへ共有する際に役立ちます。
3. 簡単なもの⇒メイン処理の順に読む
3つ目の選択肢は、プルリクを地道に読みます。
レビューイと都合がつかないなどの、会話の選択肢が取れない苦肉の策だと思っています。
自分の場合、小さいプルリクでは表示されているファイルの上から順に見ます。プルリクが小さいため、それでも内容を把握できます。大きなプルリクではそうはいきません。まず、独立した簡単なものから見ていきます。これは、メインの処理にある最も複雑なコンテキストを理解する際のノイズを最初に除去するイメージです。 CSSの簡単な変更だったり、単純な変数名の変更だったりです。
ノイズの除去後は、メイン処理をプログラムの実行順に上から読んでいきます。 過去に、実装するときは上から順に、レビューするときはバラバラ、という時期がありました。しかし、レビューするときも上から順に読むほうが理解しやすいです。
将来の巨大なプルリクエストを避ける
ここまででレビューは終わりです。
最後に、巨大なプルリクエストを分割する方法があったかを、レビューを通して得られたプルリクの理解をもとに考えます。 何度も言いますが、大きなプルリクエストのレビューは大変です。その体験を通して、どうすれば分割できるのかノウハウが溜まっていくはずです。 ノウハウをレビューイにも共有して、適切な粒度のプルリクエストをチームの文化にしていきます。
