こんにちは！リンクアンドモチベーションのモチベーションクラウド開発チームで、バックエンド開発をしている谷です。
エンジニアなら誰しも、改修が終わって自信満々で出したプルリクエストに、尋常でない数の指摘がついてしまい、落ち込んだ経験があるのではないでしょうか。
また、そのような話を周りから聞く中で、自分のプルリクエストに自信が持てず、「レビューに出すのが怖い！」という新人エンジニアの方もいらっしゃることと思います。
今回はコードレビューに苦手意識のある方向けに、私自身も経験した「ありがちな失敗」と、その対策としてチェックすべきポイントをまとめてみました。
※この記事は モチベーションクラウドシリーズ Advent Calendar 2021 の9日目の記事です。
TL;DR
- レビュワーがレビューできるように、前提情報を共有しよう
- プルリクエストの単位は、できるだけ細分化しよう
- 当たり前のことは、自分で確認しよう
- セルフチェック観点表を作成し、アップデートし続けよう
ありがちな失敗
私は2019年に新卒入社後、半年の研修を終え、モチベーションクラウド開発チームに配属され、開発に携わるようになりました。
しかし、実際開発業務に携わってみると、1人で黙々と実装していた研修の頃とは違い、先輩エンジニアに実装内容をレビューしてもらった内容を修正しながら進める必要がありました。
この時に直面したのが「実装だけでなく、それ以前の問題も含めてめちゃくちゃ指摘される」と言う問題でした。笑
具体的には、
- プルリクエストで改修する目的・方針が不明確で、レビュワーに何度も確認される
- 1プルリクエストあたりの改修内容が多すぎて、レビューバックされるまでのリードタイムが伸びる
- プロジェクト内で決めている最低限のチェック項目を確認していなくて、レビュワーに注意される
- 同じような指摘を何度もレビュワーに指摘されてしまう
など、大変お恥ずかしい限りですが、ここに上げ切れないくらい多くのご指摘をいただきました。
何度もレビュー依頼して指摘をもらい、レビュワーの時間を奪ってしまうことはとても申し訳なく感じてしまいますよね。
また、自分の至っていないところを指摘されるたびに、自分自身も傷ついていきますよね。
（私もプルリクエストのコメント数が200近くになってしまい、泣きそうになったことがあります。。。）
そこで今回は、上にあげた4点を取り上げ、どうしたらこんな悲しい思いをせずに済むのか、見ていきたいと思います。
レビュワーがレビューできるように、前提情報を共有しよう
あなたの作ったプルリクエストには、レビュワーに必要な情報が十分に記載されていますか？
レビュワーからの指摘が「実装に関する指摘」だと思ったら、実は「要件/設計の確認」だったりしませんか？
基本的にレビュワーはあなたがどんな要件の、どんな設計に基づく実装をしているのか知らないと思った方が良いでしょう。
場合によってはあなたが実装をしているリポジトリの技術スタック・コーディングルールを知らない場合もあるでしょう。
また、どんな詳細設計・実装方針で改修しようとしているのか、コードを読めばわかる場合もありますが、先に文字に起こしておいて言葉で伝えた方が、正確に伝わります。
このように、「レビュワーがレビューするにあたって必要な情報を準備し、レビューに入りやすい状態にしてあげる」ことで、レビュワーが確認に要していたコミュニケーションを断然減らすことができます。
具体的には、
- プルリクエストで取り組む実装で実現したいこと（スコープ）を概要欄に明記する
- 詳細設計・実装方針を概要欄に明記する
- 開発中の機能の要件定義書や設計書があれば概要欄に記載し、すぐに参照できる状態にする
と良いでしょう。
私達の所属しているチームでは、レビュイーがレビュワーに情報を共有しやすいよう、下記のようなプルリクエストのテンプレートをデフォルトで用意しておくことで、情報共有しやすい仕組みを作っています。
プルリクエストの単位は、できるだけ細分化しよう
楽だからとついついいろいろな改修を混ぜ込んでしまい、どのコードで何の改修をしているのかよくわからないプルリクエストを作ってしまうことはありませんか？
気付いたらプルリクエストの差分行数が膨れ上がって、読み解くのにとても時間がかかってしまう、とレビュワーに言われることはありませんか？
そんな場合は、思い切ってコードを分割し、複数のプルリクエストに分けてレビュー依頼しましょう。
よく知られているプログラミングの原則に「単一責任の原則」というものがありますが、これはプルリクエストにおいても同じだと思います。
すなわち、「1プルリクエストあたりで実装するコードの目的は、1つに絞る」ことで、プルリクエストでの改修目的が1つに絞られ明確になり、レビュワーがコードを読み解くのにかかる時間を大きく下げることができます。
さらに、プルリクエストが分割できるためには、分割できるような疎結合なコードを設計しておく必要があるので、コード品質が向上すると言う効果もあります。
分割を検討するタイミングとしては
- 1プルリクエストに対し、複数の目的の改修が混ざりそうな時
- 1プルリクエストあたりの差分が200行、多くても300行を超えそうな時
あたりに基準をおくと、レビュワーが負荷なく読むのにちょうどいいサイズに分割できると思います
（※Googleのガイドでは200行を基準にしていました。300行というのは筆者がギリギリ許せる行数です）
当たり前のことは、自分で確認しよう
自分でもやろうと思えばチェックできることを、レビュワー任せにしていませんか？
レビューしてもらう貴重な時間を、レベルの低い指摘に費やしてしまったりしていませんか？
自分でも確認できる項目をレビュワーにレビューさせてしまうということは「自分ではチェックできないが、レビュワーにチェックできること」をチェックできる時間が減ってしまうということです。
レビュワーの時間を節約し、よりレベルの高い指摘をもらうためにも、自分でチェックできることは自分で確認しましょう。
具体的には、
- コーディングに使う言語やフレームワークにおける「お作法」（文法や、命名規則などの慣習）を満たしているか
- PJT内で共通の「お作法」（コーディング規約・レビュー観点リストなど）を満たしているか
- 静的解析ツールがある場合、静的解析ツールのチェックがパスしているか（Railsの場合rubocop/RSpecなど）
は、レビュワーがいなくとも自分で、または仕組み化して自動でチェックできるようにしましょう
セルフチェック観点表を作成し、アップデートし続けよう
過去にレビュワーからもらった指摘と同じ指摘を何度も受けていることはありませんか？
指摘してもらった内容を元に、プルリクエストの出し方やコーディングを改善し実践していくことで、次第に無意識のうちにできるようになっていくものですが、日々増え続ける改善ポイントを記憶しておいてそれを毎回実践できる人は、よほど優秀でない限りいないと思います。
そこで、「セルフチェック観点表」を活用してみましょう！
具体的には
- 過去にレビュワーから指摘された改善ポイントや、一般的に良いとされているコーディングのノウハウを蓄積するための「セルフチェック観点表」を用意する
- レビューを依頼する前に「セルフチェック観点表」を一読し、自分の書いたコードが観点を満たしているか確認する
- レビュワーから新規の改善ポイントを指摘されたら、「セルフチェック観点表」に追記する
- 習慣化されて、意識する必要のなくなった観点は「セルフチェック観点表」から削除する
というサイクルを毎プルリクエストごとに繰り返し、「自分の改善ポイントとアクションを可視化し、習慣的に実践する」ことで、改善ポイントが無意識に実践できるようになるまで繰り返します。
極論、レビューなど通さなくても最高の質のコードを書ける(= 一発でapproveをもらえる)のが優秀なエンジニアです。
一度もらった指摘を二度と繰り返さないよう、習慣の力でモノにしていきましょう。
※下記に、自分が新人の頃から使っている観点チェックリストの一部を共有します
まとめ
本記事では、私自身も経験した「ありがちな失敗」とその対策としてチェックすべきポイントをまとめ、コードレビューを依頼するのに苦手意識のある方が安心してレビューを依頼できるノウハウをまとめました。
この記事を通じて、1人でもコードレビューが不安になるエンジニアと、終わらないレビューに追われるレビュワーが楽になることを祈っています！
※私もまだまだ未熟な部分の多い若手エンジニアですので、「こうするともっと良くなるよ」「それは違うよ」などのご意見あれば、是非コメントをお願いいたします！
一緒に成長していきましょう！
関連記事
プルリクエストやレビュー関連の過去記事もございますので、お時間ありましたら是非ご覧ください！