仕事術
第1回 コードレビューのフロー

ピクセルグリッドでは、成果物の品質の向上、エンジニアの技術力の向上を目指してコードレビューを導入しています。第1回目は複数人で受託案件を担当する際のコードレビューのフローを紹介します。

2014年09月18日発行

目次

本シリーズではピクセルグリッドのメンバーが、仕事をしていく上で行っているさまざまな取り組みや、その中で利用しているサービスやツールなどを紹介していきます。単なるサービスやツールの紹介だけではなく、そのツールを選んだ理由や、どのように活用しているかをお伝えできたらと思っています。

第1回の今回はピクセルグリッドが行っているコードレビューのフローを紹介します。

なぜコードレビューをするのか

まずはコードレビューをする目的について整理しましょう。コードレビューを行う目的はチームの形態や目的などによっても左右されるでしょうが、ピクセルグリッドでは次の2点を主な目的としています。

  • 成果物の品質の向上
  • エンジニアの技術力の向上

成果物の品質の向上

ピクセルグリッドの業務は受託案件が主で、2〜3人くらいのチームでひとつの案件に関わることが多くなっています。プログラムは人が書くものなので、人によって品質のばらつきはありますし、バグも必ず発生します。しかし会社としては、できるだけすべての案件の成果物の品質は同程度で納品したいですし、バグのない状態で納品する必要があります。

コードを書く人とは別の人がコードをチェックすることで、バグの早期発見、全体の設計の改善、コードの読みやすさやメンテナンス性の向上など、成果物の品質をより高いものにしたいというのが、まずひとつ目のコードレビューを行うモチベーションです。

エンジニアの技術力の向上

品質の向上だけでなく、コードレビューにはエンジニアの技術力向上という狙いもあります。

コードレビューを行うことで、レビュイー(レビューされる方)は、自分のコードについて指摘されることで新しい気付きがありますし、レビュアー(レビューする方)はレビュイーが書いたコードを読むことで、新しい気付きがある可能性があります。

また、「ここはこうしたほうがいいんじゃないか」、「いやこっちのほうがもっといいんじゃないか」というように、コードについて議論が広がることで、双方が勉強になることもあります。

Pull Requestとコードレビュー

ピクセルグリッドではソースコードの管理をGitHub(プライベートリポジトリ)で行っており、コードレビューにもGitHubを利用しています。GitHubには、Pull Requestという機能があり、これはコードレビューと非常に相性のいい機能です。

Pull Requestは、元のコードに対してこのような変更をしたので取り込んでください、というリクエストを行う機能です。Pull Requestはブランチ単位で送ることができるので、新しく機能を追加するときにブランチを切って、Pull Requestを元のブランチに送り、レビューしてOKが出た時点で、マージするというフローでコードレビューを円滑に回すことができます。

また、GitHubのPull Requestの画面では、ソースコードの1行1行に対してコメントができるので、どの行に対するレビューなのかひと目でわかり、コードレビューが非常にやりやすくなっています。

受託開発のワークフロー

ピクセルグリッドではPull Requestを使ったコードレビューのフローを、GitHubで採用されているブランチの管理フローである「GitHub Flow」を少しアレンジしたかたちで利用しています。GitHub Flowは簡単に解説すると、次のようなフローとなっています。

  1. masterブランチは常にデプロイ可能な最新の状態にしておく
  2. すべての機能追加はmasterブランチからブランチを切って作業する
  3. 作業が完了したり、助言が必要な場合はmasterブランチにPull Requestを出す
  4. 誰かがレビューし、OKであればmasterにマージし、デプロイする

GitHub Flowのほかにはgit-flowが有名ですが、git-flowはGitHub Flowと比べるとずいぶん複雑なので、リリースの頻度が高いWebサービスなどではGitHub Flowが適していると私は考えています。

最初はこのGitHub Flowをそのまま利用することを考えていましたが、それにはいくつかの問題があることがわかりました。運用のフェーズに入っているGitHubなどのサービスの開発ではGitHub Flowは有効ですが、ピクセルグリッドの受託開発はゼロから何かを作成することがほとんどです。

そういったケースでは、最初のうちはほとんどの機能がまだ開発段階であり、masterブランチが常にリリース可能というのが、まず不可能です(masterブランチがリリース可能になるのは多くの場合、納品時でしょう)。

次に新規作成のフェーズでは多くの機能がシーケンシャルに開発されるので、レビューを待ってマージというフローだと、レビューを待っている間は次の開発ができず、開発のスピードが落ちてしまうという問題がありました。

そこでGitHub Flowを一部改変し、次のようなフローでPull Requestとレビューのフローを行うようにしました。

  1. メインで開発を進めるブランチをメインストリームのブランチと定める
  2. すべての機能追加はメインストリームのブランチからブランチを切って作業する
  3. 作業が完了したり、助言が必要な場合はメインストリームのブランチにPull Requestを出す
  4. 作業が完了したら作業者がすぐにマージしてよい
  5. レビュアーは後からレビューを行い、レビュイーはレビューの結果により修正が必要であれば別ブランチを切って作業を行う

1〜3は、ほとんどGitHub Flowと同じです。多くの場合、メインストリームのブランチというのはmasterブランチですが、案件によってはmasterブランチはクライアント確認用で、開発のメインストリームは別でdevブランチを切るなどのケースもあるので、masterブランチに固定していないというだけです。GitHub Flowと大きく異なるのは4と5です。

4は、GitHub Flowが必ずレビューが通っていないとマージできないの対して、作業者(機能を作ってPull Requestした人)がすぐにマージしてよいということにしています。これはメインストリームにマージされたコードがデプロイ可能であったり納品可能な状態ではないという前提なので可能なフローです。

また、マージする前にレビューしてもらいたい場合は、作業者はすぐにマージせずにレビューを待ってマージしてもいいということにしています。

そして5でレビュアーがレビューを行うという流れです。マージした後にレビューを行うというのが、このフローの特徴的なところです。これによって、レビュー待ちによるスピードの低下を防ぎ、レビュアーも好きなときにレビューができるようになっています。

マージした後にレビューを行うので、レビューで発生した修正点などは再度メインストリームのブランチからブランチを切って作業し、同じ流れでPull Request、マージ、レビューを行います。

このワークフローの問題点

もちろんこのフローにはGitHub Flowと比べて問題点はいくつかあります。

  1. 運用フェーズでは使いづらい
  2. レビューが終了したかどうかわかりにくい

1は当たり前ですが、このフローはまだリリース前の開発時に最適化したものです。ですから運用フェーズに入ってmasterを常にリリース可能な最新な状態に保つことができるようになったら、GitHub Flowに切り替えるべきです。

2は、GitHub FlowだとPull RequestがマージされたらそのPull Requestのレビューと作業は完了というわかりやすい指針があります。しかし、このフローの場合はどこでレビューが終了したのかがわかりません。また、レビューで指摘した箇所がすべて修正されたかというチェックもしづらいです。

これはGitHubのPull Requestを少し通常と違う方法で利用しているので仕方ないことですが、レビューが終わったらレビュアーは必ず終了したという旨のコメントを付けたり、レビューで指摘されたことをチェックボックス*として洗い出し、完了したらチェックを入れて、修正の完了を明示的にするなど、ルールを決めることで解決すればよいと思っています。

*注:チェックボックス

GitHubで使えるMarkdownの書式(GitHub Flavored Markdown)にはチェックボックスがあり、タスクリストなどに利用できます。

実際、しばらくこのフローを運用してみて、この問題点がそこまで大きな問題になることはありませんでした。それよりもスピートを落とさずに開発ができるという利点のほうが大きいと感じています。

コードレビューを導入するまで

最初にコードレビューを導入するにあたり難しかったのは、どのようにこのフローに慣れてもらうかということでした。

いきなりPull Requestによるフローとレビューを同時に行うのはなかなか難しく、全員がこのフローをいきなり導入するのはなかなか難しいと思い、まずはPull Requestのフローだけに慣れてもらうことにしました。

小さな機能追加や修正もPull Requestを作って進めてもらい、すべてのPull Requestに対して私がレビューを行うというルールでまずはスタートしました。

最近ではPull Requestを行うというフローは浸透してきたので、案件ごとに、作業者同士がお互いのコードをレビューするというフェーズに移行中です。

まとめ

今回はピクセルグリッドが行っているコードレビューのフローについて解説しました。

実際には、今回紹介したようなことがすべて完璧に回っているわけではありません。もちろんいくつかの問題はコードレビューを導入したことによって改善され始めていますが、まだまだ会社の文化として浸透しているとは言えないですし、フローにも改善の余地があると感じています。今後も改善を続け、よりよいワークフローを目指していきたいと思っています。

書籍やブログなどで解説されているコードレビューのフローとは少し違う手法ですが、状況によって手法はさまざまだと思いますので、こういう方法もあるのだなという参考にしていただければと思います。

外村 和仁
外村 和仁
フロントエンド・エンジニア

HTMLコーダー、JavaScriptプログラマ、PHP、Perlのプログラマといった職務を経験後、2010年に株式会社ピクセルグリッドに入社。大規模サイトの運用、開発の経験を活かしてバックエンドからフロントエンドまで幅広く担当。2015年、退社。好きな言語はJavaScriptとRuby。 著書に『ノンプログラマのためのJavaScriptはじめの一歩』(単著:技術評論社、2012年11月7日)があり、共著も多数。このほか、WEB+DB PRESS、Software Designなどにも寄稿。