フロントエンドエンジニアをしている瀧です。 フロントエンドとは、ユーザーが直接目にする画面や動きの部分のことで、ユーザーインタフェースや、ユーザー体験を開発しています。 この記事では私のチームで行っているコードレビューについて紹介します。
瀧 章倫（タキ アキノリ）はじめに
コードレビューとは開発工程の１つで、自分が書いたコードを見てもらったり他人が書いたコードを見たりする仕事のことです。
レビューと聞くと印象的には評価・評論・批評といった感じですよね。粗探しをされているようで苦手意識がある方もいると思うのですが、うまく回れば良い点がたくさんあります。 実は、私がコードレビューを意識して時間を取るようになったのはここ数年の話です。理由は、レビューをチェック作業だと捉えていたことや、何かつまずいたり困ったことがあっても、自力で解決することがもっとも成長につながると考えていたからです。
レビューは開発プロセスの中でも重要なポイントですが、必須の工程ではないのでレビューを行っているチームもあれば、行っていないチームもあるのではと思います。 本記事を通じて、レビューの価値を少しでも感じて頂ければ幸いです。
コードレビューの流れ
私のチームではこのような流れでコードレビューを行っています。以下、もう少し詳しく紹介します。
コードレビューの粒度
私のチームではコード管理をGitLabで行っており、コードレビューはissue（課題）ドリブンで行います。issueの粒度が大きければコード量が肥大化し、レビューする側の負荷が高くなるため、issueを分解するか、区切りの良いところで複数回マージリクエストをするようにしています。GitLabではレビュー依頼のことをマージリクエストと呼んでいます。
画面実装でよくあるパターンは、以下のような単位で3回に分けてマージリクエストをする場合もありますし、枯れた実装や、チームの習熟度が高い場合は1回でする場合もあります。
- 画面を構成する部品
- 部品を組み込んだ画面
- 画面にデータを渡す処理
コードレビューのやりかた
はじめに、レビューをしてもらいたい人がマージリクエストをします。 マージリクエストには以下が書かれています。
- どのissueに紐づいたものか
- やったこと
- やったことを確認する方法
他にも懸念点があれば記載することもあります。 レビューはマージリクエストの内容から、以下の点をチェックしていきます。
- 動作が仕様を満たしているか
- 見た目がデザインと一致するか
- コードが読みやすいか
1.の仕様や2.見た目については、正解があるので人によってブレることは少ないのですが、問題は3.で、チームの文化や技術レベルに大きく依存してくるものであり、コードの読みやすさが観点が入ってくるためブレやすいです。
コードを見る時に指摘するポイントは、大きくはこの2つです。
| 指摘ポイント | 詳細 |
|---|---|
| 明らかな誤り | typo バグ 実装漏れ デザイン崩れ |
| 直した方が良くなる実装 | 不自然な命名 冗長なコード コードの重複 深すぎる分岐 長すぎる関数 大きすぎるスコープ 複雑なロジックに対する単体テストがない パフォーマンスに影響が出そうなコード ...etc |
明らかな誤りに関してはツールでもチェックできますし、デバッグすることで検出できるのですが、直した方が良くなる実装は指摘が難しいところです。 知識や経験の差が出る部分で、レビューをする側にとっては当たり前のことでも 受ける側にとっては理解できなかったり、知らなかったりすることもあります。 また、こうした方が良くはなるけど修正すると影響範囲が大きくなってしまうなど 一概にこうした方がいいと決めることが難しいものも多いため、すり合わせていく必要があります。
レビューをした人は自分が指摘した内容が全て解消されたらLGTM（Looks Good To Me= 自分が見た限りは問題ないと思う）とコメントし、最後に、レビューを受けた人がマージしてissueをcloseします。
コードレビューの成否は環境依存
身も蓋もない話ですが、コードレビューがうまく機能するかのほとんどは環境に依存します。
チームメンバーがコードレビューに価値を感じていなかったり、優先順位を下げたりするのは以下いずれかの要因を1つ以上抱えているケースがほとんどだと思います。
| 要因 | 説明 |
|---|---|
| スケジュールが厳しすぎる。マルチタスクが多く、プロジェクトに集中できない。 | 忙しさ度合いによってレビューが厳しく、諦めても仕方がないレベルです。 |
| 元のコードが複雑すぎる。 | 元のコードの仕様知識が必要。影響範囲が広かったり制約も多いので、読みやすさが犠牲になりやすく、かなり効果が出にくいです。 |
| チームが少人数。メンバーの技術力の偏りが大きい。 | 効果が出るまで時間がかかるか、効果が出にくいです。 |
| コードレビューのやり方を知らない。 | メンバーの合意を得ればできます。 |
これらの要因が少なければ少ないほど、コードレビューはうまく働きます。難易度が高すぎればそのプロジェクトではレビューを諦めるのも良いと思います。
また、難易度が低い場合でもうまくいかないケースとして、レビューはする・受けるの属性があるのでレビューを受ける側が過度に萎縮してしまったり、レビューをする側が攻撃的になってしまったりする可能性があります。 この形が改善されなければ、むしろレビューはマイナス方向に働くでしょう。
コードレビューをしてみて
実感しているメリットは、おもに品質の向上です。
- プロジェクト全体のコードのルール、品質が一定上担保される
- フィードバックのタイミングが早いので、手戻りが少なく指摘を受け入れやすい
- マージリクエストの粒度を適切に保てれば、レビューをする側の負荷も少なく問題を見つけやすい
- マージリクエスト、コードレビューのログが残るので、仕様トレーサビリティが高くなる
- 見られるプレッシャーがかかるため、雑なコードになりにくい
デメリットは、チームで歩調を合わせていくので非効率を受容しないといけない点です。
- レビューをする側は、レビュー依頼が多いと手持ちの作業が止まってしまう
- レビューを受ける側は、待ち時間が発生する
また、レビュースキル（違和感を見つける、伝える、質問する）は実装とは違うスキルが必要で、精神的に疲れたりストレスが溜まりやすいです。
さいごに
ここまで主にコードレビューのやり方について書いてきましたが、重要なのは 「なんのためにコードレビューをするのか」だと思います。
コードレビューをする意味は、個のこだわりをなくすことだと思っています。 もちろん自分の仕事にこだわりを持つことは素晴らしいことですが、突き詰めると個の依存が大きくなるということです。
「この機能は○○さんが詳しい」とか、「このアプリは○○さんしか触れない」といった属人性を防ぐには、レビューを通してプロセスの中で個人から責任を引き剥がすのが無理のない形だと思います。
責任が分散された分、すべてにこだわりがなくなるのではないか？という懸念が生まれるかもしれませんが、実際は部分に対してこだわりがなくなると全体に対してこだわりが生まれやすいです。 おそらく関わりの広さによって、全体への当事者意識が醸成されていくのだと思います。 つまり、コードレビューとは自分が書いたコードを、チームで書いたコードにする仕事とも言えます。 そう考えると、コードレビューのデメリットとして挙げたことは、将来払うであろう引き継ぎコストを先に払っているだけで、割のいい投資といえるのではないでしょうか。
