チーム開発の現場に git を導入し、git-flow, github-flow などの開発フローに則って、 pull-request とコードレビューを実施している現場も多くあるだろう。
私達のチームもその例に漏れていない。git-flow, github-flow こそ採用はしていないものの、それらをより簡略化、シンプルにしたフローで開発を行ってる。
さて、あなたのチームではコードレビューの後にPull Requestをmergeするのは誰の作業だろうか？レビューをした人？それともレビューを依頼した人？あるいは、ソースコード統括管理担当者？
私達のチームでは、Pull Requestを作った人(つまり、コードレビューを依頼した人)がソースコードのマージを行うようにルールを作成した。今回は、そのようなルールを導入している理由とメリットや効果について纏る。
レビューに通過！さあ、マージしよう・・・えっ！？
おめでとう、君の作った Pull Request は無事に自動テストに合格し、厳しいことで有名な先輩エンジニアのレビューに合格した。張り切って Merge pull request を押す・・・その前に確認をして欲しい。マージ先のブランチは君がトピックブランチを作ったときから進んでいないだろうか？
チーム開発を行っているとよくある現象だ。自分が自分で作ったブランチで作業をしている間にも、開発ブランチの作業はどんどん進んでいっている。今、そのままマージをするのは私は次の理由でオススメをしない。
- あとからリビジョンツリーを見た時にブランチの関係性が見難くなる。そのため、仮にマージコミットを revert するときに操作が複雑になりえる
- 開発ブランチでの変更によって、トピックブランチがバグを引き起こす可能性がある。
開発ブランチの変更と作業ブランチの変更が、一見関係のない部分のように見えても、思いもしないバグを引き起こす可能性がある。残念ながら、それがソフトウェアだ。
そこで、私達は開発ブランチの変更をトピックブランチへ取り込む作業を行う。このとき、 merge にするのか rebase にするのかという議論があるようだが、私達は rebase を利用している。
rebase が完了したら悪名高き force push を行う。あとは、自動テストが実施され問題が発生しないことを確認する。
さぁ今度こそマージだ・・・！！誰が・・・？
さて、いよいよ今回の本題だ。前にも書いたように私達は Pull Request を作った本人がマージを行うようにルールを作って運用している。この方法にはメリットがある。
自分に割り当てられている Issue を自分で終了できる
Pull Request と Issue が結びついていることが前提だけれど、自分に割り当てられている Issue を自分の責任で終了できるので、チームメンバーで Issue の共有をするときに、認識の齟齬が生まれにくくなる。
rebaseとの相性が良い
前の節で私達はマージの前に必ず rebase を行うことを紹介したけれど、 rebase は force push を伴う作業だ。一つのトピックブランチに2人以上の人間が force push を行うことは、コンフリクトを引き起こす一番の原因だ。問題を発生させる可能性は避けるべきであると言える。
責任の所在がはっきりとする
レビューを行うのも人間だし、マージを実施するのも人間だ。人間の行うことには残念ながらミスがつきまとう。ミスは仕方ないものとしても、再発を防止するのは必要だ。
このとき、誰がやったことが原因で生じた問題なのかをはっきりとしておくことで、原因の分析ができてその積み重ねが解決へ結びつく。
そして、最後に携わるレビュワーよりも、一番コードを書いたであろうレビュー依頼者の方が、コードの全容を知っているという意味で後者が負うのが適切だと考えている。
まとめ
私達が運用している Pull Request のルールについて纏めた。実はこのルールは、今のチームでは私が作った物では無いのだけれど、実は私が以前いたチームでも同じことをやっていたので、なんだかｽｯと馴染んだ。
まとめに書く内容となって恐縮なのだけれど、この方法は「Pull Request に対して自動でテストを行っていること」や「Pull Request と Issue が紐付いていること」が前提条件になっている。全てのチームにおいて、この方法が最適ではないのが残念だ。
ちなみに、私達のチームでは最近はコードレビューを自分の作業よりも優先するルールも取り入れた。その辺りの話はコードレビューを優先するか自分の作業を優先するかを参考にしてみて欲しい。