最近コードレビューをどのように回していくかについて考えたことがあったのでブログに書いておく。
コードレビューの目的
コードレビューには誤りの発見以外にいろいろな目的がある。自分の中ではid:hisaichi5518が昔プレゼンでまとめていた目的が結構しっくり来ている。
- https://speakerdeck.com/hisaichi5518/kodorebiyufalsehua?slide=8
- http://hisaichi5518.hatenablog.jp/entry/2014/10/29/165721
- 機械的に発見できない誤りの発見
- 技術力の向上
- 属人性の排除
コードレビューの目的としては誤りの発見と同様に、技術力の向上や属人性の排除といった教育的側面も重要である。
コードレビューで課題に思っていたこと
自分のチームでは基本的に一人がコードレビューをして、OKだったらmergeをして良いという運用だった。ただし、チームにずっといる人(ベテラン)とチームに新しく入った人(新人)が混ざっている場合に以下のような課題があった
- レビューするコードへの知識がないと、誤りを発見することが難しい
- 誤りの発見が出来ないのでは、と感じることで新人がコードレビューを躊躇する
- 結果としてベテランがコードレビューをし続け、負担が大きい
- またコードレビューによる教育的側面があまり満たされないため、その後もベテランがコードレビューし続ける
コードレビューを段階的に行ってもらう
上のような課題の解決として、PullRequestを投げた人以外の全メンバーがコードレビューをするということも考えられる。ただ、それでは教育的な側面を担保するためにコストが多すぎる。
そこで、教育的側面を満たしながら、誤りの発見をおろそかにしないように、新人にはコードレビューを段階的に行ってもらうという解決方法を取ることにした。
段階1 : 新人にレビューはしてもらうが、mergeするにはベテランのレビューが必要
最初は、
- 新人にもいろいろとレビューをしてもらう
- ただし、新人のレビューだけではPull Requestをmergeしてはならず、mergeするにはベテランのレビューが必要
という段階を取った。また、この時はレビューをしてもらうだけではなく、わからないことがあったらとにかく質問してもらうくらいで良い、ということを意識してもらった。
この段階は、新人には教育的側面のメリットを受けてもらい、ベテランが誤りの発見の責任を持つ、というスタイルである。
段階2 : 本番で動かないコードであれば、新人のレビューだけでmergeしてよい
少し慣れてきたなーと感じたら、次の段階に進む。
- 本番で動かないコードなら新人のレビューだけでmergeしてよい
- 本番で動かないコードとは、テストとか、ちょっとした便利スクリプトとか、開発環境でしか使わないコードとかそういうもの
- ただし、本番に出るコードはまだベテランのレビューを受けないとmergeしてはいけない
この段階は、誤りの発見の責任を少しずつ新人に委譲していき、ベテランの負荷を下げていくという段階である。
段階3 : 一人のレビューでmergeしてよい
さらに進んで、大分コードへの知識が溜まってきたなーと感じたら、最終段階として全ての制約を取り払って一人のレビューでmergeして良いところに進む。これで、ベテランと同じようにレビューをして良いという状態となる。
この段階でレビューが出来る人が一人増えるので、ベテランの負荷も下がる。
もちろん最終段階に進んだとしても、このコードは非常に重要だから二人くらい見たほうが良いとか、この部分の知識は浅いから別の人に見てもらおうとか、柔軟に取り扱っている。
まとめ
今回はコードレビューで課題に思っていたことに対して、コードレビューを段階的に行ってもらうというアプローチを取ったことを紹介した。個人的には段階的にコードレビューをしてもらうことで教育的側面が満たされやすく、結果としてレビュワーが増えるので、ベテランの負荷が少しずつ下がっていくだろうと思っている。