新卒エンジニアがPRを出す前にやること・チェックすること

5分で読める テック

はじめに

新卒でエンジニアとして働き始めたころ、PRを出すたびに細かいレビューコメントをもらっていました。

「console.log残ってるよ」「N+1になってない?」「masterの最新取り込んだ?」

最初はそれが普通だと思っていたのですが、あるときシニアエンジニアの先輩から言われた一言が刺さりました。

「自分でレビューできるようになってから、人にレビューを頼もう」

それからは、PRを出す前に自分でチェックする習慣をつけるようにしました。この記事では、そのチェックリストと、実際に新卒1年目で踏んだ失敗から得た教訓をまとめます。

チェックリスト(コピペして使えます)

## PRを出す前のチェックリスト

### 動作確認
- [ ] 正常系でエラーが出ないか
- [ ] 変数に予期しない値が入る可能性はないか(nullや空文字列を含む)
- [ ] 実装箇所でページが壊れていないか(PC・スマホ両方)

### コードの品質
- [ ] N+1が発生しそうな箇所はないか
- [ ] テストは書かれているか(モデルの公開メソッド等)
- [ ] 書いたテストが通っているか

### 見た目
- [ ] typoはないか
- [ ] インデントは正常か
- [ ] デバッグ用のコードが残っていないか(console.log, pry, debugger など)
- [ ] 不要なコメントアウトはないか
- [ ] 不要な空行はないか

### Git
- [ ] 最新のmainブランチを取り込んだか
- [ ] レビュワーをアサインしたか

失敗談から学んだこと

1. 「全部に適用」を確認していなかった

入社して最初のうちに、特定の画面に機能を追加する実装を担当しました。

チケットには「編集画面に共通項目を表示する」と書いてありました。ディレクターレビューも通って、実装も完了。PRを出してマージしました。

しかし後日、別の画面でその機能が動いていないという報告が来ました。

調査してみると、チケットの要件は「すべての小カテゴリ編集画面に共通項目を表示する」という意味だったのに、自分は「既存の特定カテゴリにのみ表示する」という実装をしていました。

チケットの表現があいまいだったこともありますが、実装を始める前に「どのケースに適用されるのか」を自分から確認すべきでした。

学んだこと: 完了条件を自分で確認する

PRを出す前に、完了条件を自分で書き出すようにしました。

例)このPRが完了したとき、以下の状態になっている
1. カテゴリAの編集画面に共通項目が表示される
2. カテゴリBの編集画面にも共通項目が表示される
3. 共通項目がない場合は何も表示されない

分岐を洗い出して、それぞれのケースを自分で確認する。チケットに書いていないケースが見つかったら、PMやディレクターに確認してから実装する。この習慣が、手戻りを大幅に減らしてくれました。

2. 実装内容をレビュワーに伝えずPRを出していた

最初のころは、PRの説明欄をほぼ空欄にして出していました。コードを読めばわかると思っていたからです。

しかし、レビュワーの立場からすると「このPRは何を・なぜ変えているのか」が一目でわかるほうが、レビューの質も速度も上がります。

PRの説明欄に書くべきこと:

  • 何を変えたのか(変更の概要)
  • なぜ変えたのか(背景・理由)
  • どうやってテストしたか
  • スクリーンショット(UIの変更がある場合)

PRを出す前に、説明欄を書くことがセルフレビューにもなります。「自分でうまく説明できない変更は、まだ整理できていない変更」だということに気づきました。

3. console.logを残したままPRを出した

恥ずかしい話ですが、デバッグ中に使ったconsole.logをそのまま残してPRを出したことがあります。何度か経験してから、PRを出す前にコードの差分を全部一度読むようにしました。

Gitの差分を自分で読むのは、意外と有効なセルフレビューです。エディタで書いているときには気づかなかったことが、差分として見ると気づきやすくなります。

# PRを出す前に差分をターミナルで確認する
git diff main

なぜチェックリストが効くのか

チェックリストのよいところは、「確認した」という事実を残せることです。

レビューでコメントをもらったとき、「自分は確認していなかった」という事実を認識するのと、「チェックリストのこの項目が抜けていた」と気づくのでは、次への改善のしやすさが違います。

また、チェックリストは育てるものでもあります。レビューで新しいコメントをもらったら、そのコメントをチェックリストに追加する。そうやって自分専用のチェックリストを育てていくと、だんだんと同じ指摘をもらう頻度が下がっていきます。

まとめ

PRを出す前のセルフレビューは、相手へのリスペクトでもあります。「自分ではこれだけ確認しました」という状態で依頼することで、レビュワーはより重要なことに集中できます。

最初から完璧にこなす必要はありません。まずは「console.logを残さない」「差分を自分で一度読む」「最新のmainを取り込む」という基本的なことから始めてみてください。

繰り返すうちに、チェックが習慣になっていきます。

チェックリスト(再掲)

- [ ] 正常系でエラーが出ないか
- [ ] 予期しない値(null、空文字)が入るケースは考慮しているか
- [ ] 実装箇所でページが壊れていないか(PC・スマホ)
- [ ] N+1が発生していないか
- [ ] テストは書かれているか・通っているか
- [ ] typoはないか
- [ ] デバッグコードが残っていないか(console.log、pry など)
- [ ] 不要なコメントアウト・空行はないか
- [ ] 最新のmainを取り込んだか
- [ ] PRの説明欄を書いたか
- [ ] レビュワーをアサインしたか

質問・リクエストを送る

記事についての質問や、取り上げてほしいテーマがあればお気軽にどうぞ。いただいた質問はブログ記事として回答し、Q&Aページで公開することがあります。