チーム共通で使うためのコードレビューガイドラインを作った

こんにちは。Zaim で開発部のマネージャーをしている高山です。Zaim ではアプリチームやバックエンドチームなど、チームごとに開発方針やコードレビューのガイドラインを作って運用しています。

チームごとに用意しているので、開発言語や環境に絞った内容が多く、特にコードレビューのガイドラインは最小限の内容です。人数が少ないうちはあまり困ることはありませんでしたが、規模が大きくなるにつれて少しずつコードレビューに対する意識のズレを感じるようになってきました。

コードレビューする際のコミュニケーションの姿勢や、どこを見るべきか、どこは無視するべきかなど、どんなチームでも同じように意識するべき基本的な事柄は多くあります。こうした情報を全チーム共通の指針としてまとめておくことで、開発者同士のコミュニケーションがより円滑になり開発スピードも向上すると考えまとめましたので、ご紹介します。

コードレビューの前提

完璧なコードを目指しすぎるとかかる時間・コストが膨大になるので、Better な状態を目指すことを大前提とします。特に、レビュアー側は完璧（Best）を求めすぎないように意識していきたいです。当然のことですが、コードの品質を下げてもいいというわけではありません。適度な品質を維持しつつ、最適な妥協点を追求します。

ただし、緊急時にはコードの品質をあと回しにしても構いません。具体的には「障害が起きていて素早く解決する必要がある」場合などです。しかしながら、こうした場合でもコードの品質の低下を放置してよいわけではありません。後日、時間を作って解消しましょう。

レビューリクエストを処理するタイミング

他の人からレビューリクエストが来た場合、できるだけ素早く対応します。遅くとも翌営業日までには何らかのリアクションはしたいです。

レビューが遅れることの弊害はいくつかありますが、レビューイのコンテキストスイッチのコストが大きくなってしまうことを、特に意識しましょう。レビューを受けるまでの期間が長引くほど、作業した内容を忘れていってしまうので、コメントで指摘を受けたときに「何だったけ？」と作業した内容を思い出す負担が大きくなります。

「レビュー → 修正 → マージ」のサイクルはできる限り素早く回すようにしていきたいです。

そもそもレビューを貯めないための工夫が大事です。コードレビューを素早く実施するために、日々の業務時間の中にあらかじめレビュー用の時間を確保しておくことをおすすめします。特に自分にアサインされているレビューリクエストは、毎朝確認しましょう。

良いレビューリクエストの出しかた

レビューリクエストをする際には最低限、次の項目を意識します。

・複数の変更理由が混在していない
・Danger などの静的解析ツールからの指摘が残っていない
・変更内容（diff）が大きくない（変更が大きい時は PR の分割を検討）
・動作テストのやり方が分かる
・仕様を記述したドキュメントやタスクへのリンクがある
・UI の変更がある場合はスクリーンショットがあると望ましい

情報を整理しておくとレビュワーの負担が少なく、素早くレビューしてもらえる傾向があります。また、何らかの事情により過去のプルリクエストを見返す際にも情報を整理しておくことは大切です。

コードレビューで見るべきところ

レビュアーは動作確認とコードチェック、仕様に沿っているのかを確認します。すべての変更内容にひととおり目を通す習慣を付けておきたいです。

動作確認

UI にからむ部分はシミュレータやローカル環境で動かすことが望ましいです。変更が小さい場合に、目視チェックに自信がある場合はその限りではありません。ただし、実際に動かしてみないとよく分からないことは多いため過信は禁物ということは忘れないでおきましょう。

特にリンク切れは見落としがちなので、リンクに変更がある場合は注意していきたいです。

コードチェック

コードの内容は、最低限次の項目は押さえます。

・命名規則は適切か
・設計がプロジェクトの指針に沿っているか
・スタイルガイドに沿ったコードになっているか
・コードが複雑になっていないか
・「将来、必要かも」みたいなコードが入ってないか
・コードの変更に合わせてユニットテストが更新されているか

命名規則や複雑さなど、複数の選択肢がありえて、どちらも間違いではない場合はコード作者の意見を尊重します。こうした内容の指摘や、単なる意見を言う場合は IMO（in my opinion = 自分の意見では）を付けて「このコメントはスルーしても良い」と表明しましょう。

コードレビュー時に「単なる意見や好みの問題」のやり取りを始めると、マージまでの時間が遅くなります。適宜、Slack や朝会など他の場でコミュニケーションするほうが望ましいこともあるので意識しておきたいです。

テストケースのカバレッジ

動作確認用の項目およびユニットテストは、適切なテストケースと実装ができているか確認します。本来、必要なテストケースが漏れていて障害につながるケースもあります。

良いコードには「良い」という

コードレビューは基本的にはあら捜しなので、悪いところばかり指摘することになります。意識的にポジティブなフィードバックを心がけるようにすると、作業にまつわる心理的な負担がグッと軽減されます。

・素直に良い（すごい）と思ったコードを見つけたとき
・以前、指摘した内容が改善されていたとき
・自分がミスってた箇所を改善してくれていたとき

コメントをせずとも GitHub では 👍 や 🎉 などのリアクションを付けられるので、こちらもおすすめです。

指摘するときは自分のことは棚に上げる

「ここを変えたほうがいいと思うけれど、自分もできていないので言えない」というのは避けます。必要な指摘はどんな立場の人間でもできる状態が健全です。

指摘されたほうは「あなたもできてないでしょ」という反論はしてはいけません。やるべきことができてない場合はチームの問題なので、チームで問題を解決していきましょう。

レビューイディオム

コードレビューまわりでよく使う略称です。活用すると、意図が伝わりやすくコミュニケーションがスムーズになります。

ユニットテスト

適切にユニットテストを用意すれば、レビューに関する主に動作テストまわりの負担を軽減できます。

たとえば、家計簿のデータから残高を集計するロジックを作った場合、計算内容が正しいものかどうかをレビュワーが手作業で動作を確認するのはかなり骨が折れます。検証に使う家計簿のデータを何種類も用意するのは、そもそも大変です。

同様に、日付が意味を持つ処理の場合も動作確認の難易度は高くなります。「月の開始日が 15 日だった場合、4 月 14 日は Zaim でいう 3 月である」といった処理が正しく動作しているかを確認するのは、機械にやらせたほうがはるかに効率的で間違いがありません。

ロジック関連のテストはできるだけ作るべきで、さらにこうした 「データの集計をする処理」と「日付がからむ処理」 を書く際には必ずテストを用意しましょう。

まとめ

長くなりましたが、以上のような内容を基準として開発に取り組んでいます。特に、これまで曖昧だった「個人の好みの問題をレビュー上でやり取りするのはどうなのか？」ということに一定の方針が示せたことは大きかったです。

Zaim では、効果的に開発を進める取り組みを常に検討して改善をしていっています。まだまだ紹介しきれていないような開発や運用の体制がありますので、よければカジュアルな面談にでもいらしてください。

開発組織としての体制作りから一緒に議論ができる人も大歓迎です。
ありがとうございました。

株式会社 Zaim・エンジニア／デザイナー／経営戦略担当を積極採用中！