Pull requestのレビュに関して考えること
チーム開発にほぼ欠かせないレビュに関して、考えをまとめておきたいと思い、筆をとりました。
インターンなどに教える際の参考資料としてまとめて書き残しておきたいという意図もあります。
理想
「端的にレビュすることでコストを最小化し、マージまでかかる時間を短く保つ」
上記は私が考えるPull request(以下PR)レビュの理想です。
心理的安全性が十分に確保された完成したチームであれば、不可能ではないと思います。 しかし、チームを完成させることの難易度は非常に高いので、理想だけではなく現実問題として考える必要があると思います。
現実に生じる問題
- コードへの指摘でなく個人攻撃される
- コードへの指摘を個人攻撃だと認識される
- 厳しすぎるレビュでマージされるまでが遅い
- 優しすぎるレビュでレビュをやる意味がない
- マージが遅くなりコンフリクト地獄でさらにレビュ項目が増える
- レビュのコストが大きすぎてレビュが開始されるまでが遅い
- etc
いろんな問題が併発するため、負のスパイラルに陥ると、コードを一切書けず進捗がないのに疲労感が半端ないなんてことがざらにあります。
上記の項目がよくないことはわかると思いますが、気付いた頃にはメンバ間に溝ができていたなんてこともあるため、よくない兆候・気をつけるべき指標を挙げます。
よくない兆候
- レビュアが偏る
特定の機能開発においてノウハウを持っている人に意図的に偏るのはある程度仕方ありません。ただ、そういった理由もなしに偏っている場合は、誰かにレビュされるのを避けるようなことが起こっているかもしれません。
- PRが連なっている
GitHubのPRコメントに「#100を先にレビュしてください」と書いてあったりします。未レビュのブランチに変更を加えてさらにPRにしていては、その前のレビュでちゃぶ台返しになることがあります。
基本的には1つレビュを終えてマージしてから次のレビュを出しましょう。 「それでは遅い」場合はレビュのレスポンスが遅くないか疑いましょう。
- レビュの必要性に疑問を持つメンバーがいる
レビュは大抵の場合において必要です。そのメンバーがネガティブメンバーなのではなく、レビュで疲弊するようなことが頻発している可能性があります。
気をつけるべき指標
- レビュ依頼からレビュが返ってくるまでの時間
- レビュ待ち状態のPR数
- 1PRあたりのレビュの往復回数
- 1PRのdiff行数
私個人の感覚値ですが、頻繁に下記のボーダーを超えるのは不健全です。
- レビュが返ってくるまで1日以下
- レビュ待ち状態のPR数 人数 * 2 以下
- 1PRあたりのレビュの往復回数 3 以下
- 1PRのdiff行数 100 以下
改善のために
前提の共有
メンバー全員にレビュの意義・生じている問題・良いレビュの定義を共有しましょう。
どんなチームであれ、真っ先にに下記の2点は共有すべきです。
- レビュアは個人攻撃してはいけない
- レビュイは個人攻撃されたと誤認しないよう努める
良いレビュを考えるにあたっては、レビュア・レビュイお互いの思いやりが大切です。
しかし「思いやり」だけでは漠然としているので、意識した方が良いことを後述します。
レビュアが意識したほうが良いこと
- 人格攻撃と受け取られにくい表現をする
- 修正を依頼する際は可能な限り理由・マージするための条件・改善案を添える
- 1PRに1つは褒める
- 個人のコーディングルールを押し付けない
- レビュイの方が技術力が上だと感じていても臆さずレビュする
- レビュが遅れる or する時間がない ときは正直に伝える
レビュイが意識したほうが良いこと
- 依頼する前にレビュ時に意識して欲しい箇所などコメントをつける
- PRの経緯・マージまでの締め切りなどレビュアが確認できるようにしておく
- 感情的な返信をしない
- 理由もわからず、納得感がないまま書き換えない
- レビュアにメンションすることを遠慮しすぎない
- 忙しそうであれば別の人をアサインする
仕組み化
指標を測定可能にする
まずは気をつけるべき指標を確認できる環境を整えましょう。
これらだけでも、最低限の測定は可能になります。
レビュリマインドbotを導入する
定期的にレビュ待ち状態のPRをレビュア(Assignees)に対してメンションするなどしましょう。
GitHubのPR機能を使用していればbotの作成はさほど難しくありませんし、例はWebに転がっていると思います。
定期的にメンションすることでコンスタントにレビュを思い出し、レビュを促進できますし、botなので「いちいちうるさいな」などと思われることもありません。
また、レビュ専用の公開チャネルにbotを導入すると、誰にレビュが溜まっているか、どの程度放置されているかが一目瞭然になります。 私のおすすめは、始業直後と夕方の1日2回です。(当然チームによる)
botがメンションするとはいえ、PR完成時に直接依頼したほうが早いし、依頼され次第すぐにレビュしたほうが良いということは頭にいれておきましょう。
Tips
- レビュアをランダムアサインする
ランダムにアサインすることで、レビュの偏りや属人化をある程度防げます。また、ランダムにすると「忙しそうだから依頼しにくいな」といった遠慮をしないで済みます。
- レビュコメントに程度を示す
私はレビュ時のコメントでは下記のように程度や意図を示しています
「MUST」必ず書き換えるべき、改善されなければマージできない(重大なインシデントを引き起こしうる、著しく可読性を下げる など)
「IMO」書き換えた方がよい緩やかな指摘(動作は問題ないが短くわかりやすく書ける など)
「nits」どっちでもいいくらいのことだけど、自分はこうするかも程度の指摘
「Q」単なる質問(「xxの場合を考慮した？」などを嫌味・皮肉抜きで確認したいため)
- コンスタントにマージする
マージされることは達成感があり、気持ちがいいです。ある程度の落とし所を見つけ、リズムよくマージしましょう。ジュニアクラスのメンバーはマージを成功体験として積み上げさせると良く成長します。マージした後こっそり書き換えるのはおすすめしません。また、致命的バグや顧客と約束した締め切りなど、とりあえず出すことがもっとも重要な場合もあります。
- 口頭でのコミュニケーションも利用する
何もGitHub上でレビュを完結させる必要はありません。テキストの表現力には限界があります。無用なすれ違いを起こしたり、レビュの往復回数が増えるよりは口頭でさくさく片付ける方が良いでしょう。口頭での合意事項をPRにコメントすることでログも残ります。
- ペアプロ・モブプロをやってみる
レビュアが偉いわけでも、一方的にスキルが上というわけでもありません。ペアプロすることでレビュアとレビュイ1人ずつでは思いつけない解決策が見つかる可能性があります。リアルタイムレビュくらいの気持ちでやってみるといいかもしれません。
総括
チーム内で共通認識を作り、仕組み化でコストを下げつつ、お互いの思いやりも忘れずにレビュしましょう
