昨今のWEB開発では透明・空気に近い存在になるぐらい、当たり前になっているバージョン管理。特に Git / GitHub を活用した開発のケースが多いと思います。
WEB開発をしていく中で、複数人のチームで開発を行うことも多いですよね。最初から最後まで一人で完結してしまうケースはあまりないのかもしれませんし、あるとすれば、おそらく、一人歩きしてしまっているWEBシステムが出来上がった経験をしている方も少なくないはず。 ← 実際に自分このケースだったし。
さて、今回はコードレビュー文化を促す「プルリクエスト」の重要性、「どのような形(粒度)でプルリクエストを出せば、レビュワーに気持ち良くレビューしてもらえるのか?」について、実際にWEB開発業務の中で気づいた点やメリットなどを書いておこうと思います。
※本記事はすでにコードレビュー文化があることを想定しています。
プルリクの粒度はより細かい方がよい
まず、結論からお話すると、「プルリクエストの粒度は細かい方が良い」に尽きます。なぜなら「レビュワーがレビューしやすいから」です。誰でもそうですが、コードを読む範囲が小さければ小さい程、作業量自体が減るので、レビュワーの負担が軽くなるのは当然です。
もし、できるだけ早く開発を進めたいから、先にある程度の量を作ってしまってまとめて出そうと考えているのであれば、もしくは既にそのような感覚で開発しているとすれば、それは「プルリクエストのアンチパターン」に入ってしまっている状態といえます。
「急がば回れ」という言葉があるように、プログラムをまとめて作ってしまっていたとすれば、ブレーキのないバイクで突っ走っているようなものです。事故って救急車(レビュワー)に運ばれてしまうのは目に見えています。(そして、結果的にお互いが辛いです。)
なので、プルリクの粒度に迷ったら、とりあえず細かく出すことを意識すること。それがベストアンサーだと思います。
次は、プルリクの粒度を細かくすることで受ける恩恵について記していきますね。
コードレビューしやすい
繰り返しますが、プルリクエストの粒度が細かい、小さいことは非常に良いことです。なぜなら、コードレビューがしやすくなるからです。コードを把握する範囲が小さければ、よりそのこと(コード)だけに集中することが出来ます。
もし、レビューしなければいけないコードの範囲が大きければ大きい程、より長い集中が必要になります。どんなベテランエンジニアでも、人間ですから、集中できる時間に限りがあるのは当然です。
また、「プルリクが細かいことが良い」と理解しているエンジニアであれば、雑に作られたプルリクエストにストレスを感じるもの。「もうちょっと細かいほうがいいんじゃね?」という気持ちになってしまえば、レビューをする気もつらくなるのは目に見えています。
できるだけ、一つの機能(メソッド単位でも可)に一つのプルリクを意識しましょう。
「その機能だけ動けばよい」という完結ごとにプルリクを出すことができれば、あなたもプルリクマスターです。
間違い探ししやすい
プルリクエストの粒度が細かければ、「コードの間違い探しをしやすい」です。レビュワーは一つの事に集中することができるので、より実装された機能に対する細かいアンテナを張ることができます。ということは想定できるケースをより多角的に考慮することができるということです。
なので、結果的により高い確率でコードの正しさの保証ができます。
個人的見解ですが、プログラムはプログラマーによってテストされる方が、より多角的なデバッグができると思っています。エンジニア同士でコードレビューし合うことで、バグが見つけやすく、「正しく動く」という保証と信頼の高いクオリティのものができると考えています。
アイデアが生まれやすい
大粒のプルリクエストを出されるより、一つの機能、一つのメソッド単位でプルリクを出してもらった方が「このプルリクで何がなされるべきなのか?」が明確になります。
結果、対象プルリクについての他のアイデアが生まれやすいです。実際にあった最近の出来事ですが、例えば、JSを使った一つの機能を開発し、機能要件自体は満たしていたと認識していますが、その作り方自体の方向性が間違っていることをレビュワーが気づいてくれました。
一旦Viewで描写したものを動的にDOMを操作すること自体をView(Template)側に任せてしまった方が、より遅い回線の(3G低速など)でテスト描写した時に、読み込みが完了するまでに酷いレイアウト崩れが発生する様になります。
なので、そういった考慮を想定した作り方をすべきと指摘をいただき、新しいアイデアとは少し違うかもですが、コードレビューをする範囲が細かいことで色んなケースでコードに対する提案が生まれやすくなります。
自分が想定しているよりもクオリティが高い機能ができる
アイデアに通ずるものになりますが、プルリク時は数コミットで出来上がっていた機能が、最終的に30コミットぐらいに膨れ上がっている場合が多々あります。
これは、まあ、自分の考慮不足だったり、とりあえず機能作って先進めちゃおうぜ作戦(いつまでもコネコネしてるよりも他のエンジニアの頭を借りてタスク達成に向かう)という意味でも、アクションを早めに起こし、レビューを貰って修正を繰り返す、結果的に想定しているよりもクオリティの高い機能が出来上がった!という感覚を得やすいです。(※お互いの達成感もあると勝手に思っている)
ただ、勘違いしないでもらいたいのは、きちんと機能が出来上がっていないのに、先に進めようとプルリクを出すということではありません。現段階で想定できる考えや、機能、テスト、それぞれしっかり達成できた。というところまで作ってプルリクを出すべきではあると思います。ただ、これ以上、自身の頭で思いつかないのに、無駄に何かが思いつくまで時間を消費してしまったりすることは工数的にも非効率なので、一旦プルリクを出す。それで、レビュー的にOKの場合もあるし、「こういう考え方もあるよ」と新しい気づきや、メンバー同士の学びの機会につながっていきます。
コードレビューはエンジニアにとって必須といっていいと思います。
まだ、コードレビュー文化に触れていない方は一度検討してみるとよいでしょう。
最後に
プルリクエストの粒度についてお話しました。いいことばかりを述べてきましたが、唯一デメリットを挙げるとすれば、プルリクを細かく出すことによって、開発作業が先に進みにくく感じてしまうことにあるのかなと思います。特に慣れない人はそう感じるでしょう。
なので、心情的にはある程度機能が出来上がってから出したい。そんな気持ちに駆られると思います。プルリクを出しつつも、作業自体は進めておき、コミットをgit cherry-pick
で引き抜いていくケースなども考えることもできますが、そもそもプルリク自体が正しい方向なのかどうかもわからない状態である以上、無駄な作業にもなってしまいかねませんね。その辺りは業務ですから、複数の案件を持ちつつ、レビュー側に回ったり、他の作業に取り組むことの方が吉。(要はケースバイケースです。それが仕事人としての肝かな)
とかく、プルクエストは粒度が細かいことに越したことはなく、長期的視点から見ても、よりクオリティの高いコードが出来上がりますし、運用もしやすいシステムができるであろうことは、実際に経験してみて、太鼓判は押しておきます。