これはなに?
チーム開発におけるPRレビュー(=コードレビュー)を効率的かつ健全に進めるために意識したい点をまとめる。
- レビュイー(実装者 && レビュー依頼する人)
- レビュワー(レビューする人)
- チーム
この3者の観点で工夫できることについて、実装からPRマージまでの一連の流れに沿って語っていく。
実装〜PR作成前フェーズ
1. commitは綺麗にしておく
commitログを頼りにレビューする人がいることや、未来に過去commitを参考したり特定commitをrevertしたりするケースもあるため、commitは綺麗にしておくのが良い。
「1つのcommitに含める変更量を小さくする」「意味のある単位でcommitをまとめる」「commitメッセージを真面目に書く」などなど。
この辺は、チームとしてルールを決めたり生成AI活用したりすることで、少ない労力で対応できる。
2. PR作成前にセルフレビューする
実装が一通り完了したら、実装者本人が変更内容を確認する。
客観的に自分の作業内容を見返すと、何かしらのミスは見つかるのでこのタイミングで潰しておく。
ミスが見つからない場合、セルフレビュー方法が悪い。(過激派発言)
「目的に沿った変更を加えていること」「関係ない箇所について変更を加えていないこと」「Linterやformatterが適用されていること」などなど。
セルフレビュー観点は、チームの開発規約や任されたタスクに応じてよしなに。
生成AIのおかげで、このセルフレビューの質はかなり向上し、実装者の負担も軽減されたのは嬉しいこと。
PR作成フェーズ
3. 差分行数が多すぎる場合はPRを分割する
レビュワー目線、PRが大きくなるにつれてレビューに必要な時間が増える。
人間の認知力には限界があるため、一度のレビューに例えば1時間以上費やすと、適切なレビューが困難になる問題がある。
加えて、レビュイー目線でも、レビュー依頼してからマージされるまでの時間が伸びてしまうことで不都合が生じる。
分かりやすいものだと「自身のPRがオープンしている間に他PRがマージされてしまうことで、修正対象にコンフリクトが発生し、その解消に追加の労力を割くことになる」といったケース。
というわけで、チームとしてPR1つあたりの差分行数の上限値を設定しておくのが吉。
具体的な行数は諸説あるが、500行など?
下記記事などが参考になるかも。
4. PRテンプレートに沿って最低限の項目を埋める
レビュイーは初手でPR概要欄を見るため、PR内容を理解するために必要な情報群を簡潔にまとめたい。
チーム開発しているならば、下記3項目は最低限PRに含めたい。
- 何の対応をしたのか?
- 要約と詳細
- 意図や背景もあると良い
- 動作確認
- どのような観点と手法で動作確認をしたのか
- 結果は?
- レビュー観点
- 細部のロジックまで見て欲しいのか、大まかに見れば十分なのか
- 特に重視してレビューして欲しい点があるか
5. PR差分画面で補足コメントつける
レビュワーの属性に応じて補足情報をつけてあげると親切。
レビュワーの負担を軽減する目的や、せっかくのレビュー機会を有効活用しようという意図。
以下、一例。
- プロジェクト参画したばかりの人がレビュワーなので、情報共有も兼ねてコメントつける
- 忙しいマネージャーがレビュワーなので、最低限見て欲しい箇所に目印としてコメントつける
- (プロジェクト歴長い人がレビュワーなので、特に補足コメントつけない、というのも大いにあり)
PRレビュー依頼フェーズ
6. レビュー期限を伝える
レビュワーも別タスクを抱えているため、依頼されたPRのレビュー優先度を考える必要がある。
その判断材料として期限は伝えるべき。
希望と必達の期限を伝えると良い。
また、マージ期限を伝えるでも良いし、初回レビュー期限を伝えるでも良い。
7. レビュワーを明示する
メンションなしでレビューお願いします、と投げたPRは基本的に誰も見ない。
これはレビュイー判断でレビュワー指定するのが難しい状況もよくある。
チームとして方針を決めておくのが良い。
ランダムbotでレビュワーを自動選出したり、持ち回りで実施したり……
ちなみに、レビュー判断でレビュワー指定する際は、レビュワーに指定した意図を伝えると親切。
PRレビューフェーズ
8. リスペクトを持ってレビューする
テキストコミュニケーションのため、表現に注意。
基本的に、対応してくれたことに感謝し、リスペクトを持ってレビューすれば問題なし。
9. 意図のわかるコメントをする
質問なのか、修正依頼なのか、ただの個人的な感想なのか、それが伝わる内容にする。
mustやwant、FYIなどのタグ(?)を活用するのも有効。
修正〜再レビュー
10. 質問に対して修正で返さない!!
レビュワーが悲しむため、質問に対してまずは返信をしたい。
質問内容を読み、これは修正が必要だと考えた場合でも、基本的には修正前にコミュニケーションをとるべき。
質問はあくまで「質問」であり、「修正依頼」ではない。
もしも「なんでこの実装にしたの?(あり得ないでしょ)」という趣旨のコメントを残すレビュワーがいたら、それはレビュワーがよろしくない。
11. 後で対応するものはタスク化しておく
「本PRで対応しなくても良いですが、このように変更したいですね」というコメントがつくことも多い。
PRに関係あるコメントであれば対応したいが、本筋から外れたコメントであれば別途対応とするほうが健全。
本筋から外れたものを色々と対応してしまうと、PR肥大化問題に繋がるため。
そういったものは、別途対応すると合意を取り、しっかりとタスク化・Issue化しておく。
タスク化しないと忘れ去られてしまうため。
マージ
レビューが完了したらマージ!
以上!
おわりに
まれによく観測される「レビュイーの手間は減っているが、レビュワーにそのしわ寄せが来る(もしくはその逆)」というケースを改善する手助けになれば嬉しい。