株式会社Good Labでエンジニアをしている コータロー です。
日々、Java・SQL・Gitなどの技術情報や、新人エンジニア向けの学習ノウハウ、
AI活用についての情報を発信しています。
Good Labについて気になった方は、コーポレートサイトもぜひご覧ください。
▶コーポレートサイト
はじめに
PRを出して、しばらくして通知が来ます。開くと、コメントが12件。
どこから手を付ければいいんだろう。とりあえず上から直すか……
この「とりあえず上から直す」が、いちばん手戻りします。
コードレビューの記事はレビューする側向けのものが大半で、受ける側(レビュイー)が何をすべきか はあまり体系化されていません。しかし実務で差が出るのは、圧倒的に受ける側の振る舞いです。
- 「言われたとおり直す人」→ レビューが3往復、4往復と伸びる
- 「レビュー1回で信頼を得る人」→ 2往復目で承認され、次から任される範囲が広がる
この差は、能力ではなく 手順 です。この記事では、コメントが飛んできた瞬間から再レビュー依頼までを、コマンド・テンプレ・チェックリストで書きます。精神論は書きません。
検証環境について
git / gh のコマンドと出力は、すべて手元で実行して確認したものです。
| 項目 | 内容 |
|---|---|
| git | 2.50.1 |
| GitHub CLI (gh) | 2.86.0 |
| 実演用リポジトリ | ローカルにbareリポジトリ(origin)+作業者2名分のクローンを作成して再現 |
| 実在PRの例 | spring-projects/spring-petclinic の公開PR |
実演用リポジトリの出力に出てくるリモートURLは、環境依存の長いローカルパスだったため /path/to/origin.git に置換しています。それ以外の出力(コミットハッシュ・エラーメッセージ・rebaseのTODOリスト等)は実行結果そのまま です。
1. 全体像:レビュー対応の5ステップ
ポイントは、Step 0 と Step 1 に修正作業が1行も含まれていないこと です。ここを飛ばすから手戻りします。
2. Step 0:まず全部読む。1つも直さない
なぜ反射的に直すと失敗するのか
| # | 起きること | 具体例 |
|---|---|---|
| 1 | 後の指摘で前の修正が無駄になる | 3件目の指摘に従って変数名を丁寧に直した後、10件目に「このクラスごと不要では?」というコメントを見つける |
| 2 | 指摘の重複・矛盾に気づけない | 別々のレビュアーが逆のことを言っている。片方だけ直すと、もう片方から再指摘される |
| 3 | 本当の論点を見落とす | 些細な体裁の指摘10件に埋もれて、設計に関する1件を後回しにしてしまう |
レビューコメントは 時系列に並んでいるだけで、重要度順ではありません。 上から順に処理するのは、優先度を放棄しているのと同じです。
「全部読む」を確実にやる
ブラウザだけで読むと、折りたたまれた古いコメントや、Filesタブにしか出ないコメントを見落とします。CLIで一覧化するのが確実です。
# PRの概要とコメントを一気に読む
gh pr view 2559 -R spring-projects/spring-petclinic --comments
実際の出力(末尾部分):
--
author: Prajwalraut29
association: contributor
edited: false
status: none
--
Done. I've added @SuppressWarnings("deprecation") and made the suggested changes. Thanks for the review!
--
author: dsyer
association: member
edited: false
status: none
--
You just need to sign your commit.
--
自分のPRなら、番号を省略してブランチから解決させることもできます。
gh pr view --comments # 現在のブランチのPR
gh pr diff --name-only # 変更ファイル一覧(レビュー範囲の再確認)
gh pr checks # CIの状態
DCO pass 1s https://probot.github.io/apps/dco/
CIが落ちている状態でレビュー依頼を出さない。 これは指摘以前の問題で、レビュアーの時間を確実に無駄にします。gh pr checks は依頼前に必ず確認してください。
3. Step 1:コメントを5つに分類する
全部読んだら、手を動かす前に分類 します。ここが、この記事でいちばん重要なパートです。
分類表
| 分類 | 見分け方 | 取るべき行動 | 期限 |
|---|---|---|---|
| A. 対応必須 | バグ・脆弱性・仕様違反・データ不整合・規約違反 | 必ず直す | このPR内 |
| B. 推奨(直したほうがよい) | 可読性・重複・テスト不足など、品質は上がるが機能は変わらない | 原則直す。規模が大きければ別チケット化を提案 | このPR or 別PR |
| C. 好みの範囲 | 「自分ならこう書く」系。どちらでも正しい | どちらでもよい。迷ったら直す(議論コストのほうが高い) | このPR |
| D. 質問 | 「これはなぜ?」「〜という理解で合ってる?」 | 返信する。コードは変えない(説明で解決するなら、それが正解) | 即レス |
| E. 認識ズレの疑い | 指摘の前提が自分の理解と食い違う | 直す前に確認する(最優先) | 着手前 |
「直す」以外の選択肢を持つ
新人ほど「指摘=全部直すもの」と思いがちですが、実務では次の4つがすべて正当な対応です。
| 選択肢 | 使いどころ | 必要なもの |
|---|---|---|
| ① このPRで直す | A・C、規模の小さいB | — |
| ② 別チケット/別PRに切り出す | 修正範囲がPRの目的を超えるB | チケットを実際に作ってリンクを貼る(口約束にしない) |
| ③ 議論する | E、設計方針に関わるB | 論点の明確化(後述のテンプレ) |
| ④ 現状維持の根拠を示す | 指摘の前提が変わっている場合 | 根拠(コード・テスト・計測・仕様) |
② を選ぶときの絶対条件は 「その場でチケットを作る」 です。「後でチケット化します」は9割やられません。レビュアーはそれを知っているので、リンクのない「別対応にします」は信用されません。逆に、チケットURLが貼ってあれば一発で通ります。
分類を可視化する
分類した結果を、PRに1コメントとして投稿してしまうのが最も速いです。レビュアーは「何がどう処理されるか」を一目で把握でき、この時点で議論が始まる ので手戻りが激減します。
コメントありがとうございます。以下の方針で対応します。着手前に方針だけご確認ください。
| # | 指摘 | 対応 |
|---|------|------|
| 1 | 検索条件がnullのとき落ちる | 直します(必須) |
| 2 | Serviceに寄せたほうがよいのでは | 直します |
| 3 | メソッド名 search → findByLastName | 直します |
| 4 | キャッシュを入れたほうがよい | 別チケットにさせてください(#432 を作成しました) |
| 5 | この分岐は不要では? | すみません、意図が読み取れていない可能性があります。個別に質問しました |
「先に方針を出す」だけで、レビュアーの体感は大きく変わります。 何も言わずに30分後に大量のコミットが飛んでくるのと、方針表が先に来るのとでは、信頼度が違います。
4. Step 2:修正の順序 ― 逆順でやると全部やり直しになる
分類できたら、必ずこの順序で直します。
① 設計・構造に関わる指摘(クラス分割、責務、レイヤー、API設計)
↓
② ロジックの誤り(境界値、null、例外、並行性、SQL)
↓
③ 命名・可読性(メソッド名、変数名、コメント、分岐の整理)
↓
④ 些細な体裁(インデント、import順、空行、typo)
逆順の悲劇
④→①でやると、こうなります。
- import順を整え、typoを直し、変数名を丁寧にリネームする(30分)
- その後、①の「このクラスはServiceに移すべき」に着手する
- クラスごと移動 → 直したはずの命名も体裁も全部やり直し
- さらに、移動後のクラスに対してレビュアーから体裁の再指摘
①が確定していないうちに②〜④をやるのは、土台が決まっていない家の壁紙を貼る ようなものです。
逆に、④は最後に一括で潰せる
体裁はほぼ機械で直せます(mvn spotless:apply / ./gradlew spotlessApply / npm run lint -- --fix / gofmt -w . など)。人間が時間を使うべきは①と②だけ です。③④に時間を溶かしているなら、それは仕組み化の失敗です(→ 第9章)。
①を直すと②〜④の指摘が自然消滅することがあります。 クラスを分割したら、指摘されていた長い分岐そのものが消える、というのはよくあります。だから①が先なのです。
5. Step 3:再レビューしやすい差分を作る
修正内容が同じでも、差分の見せ方 で再レビューのコストは何倍も変わります。ここは完全に技術の話なので、そのまま真似できます。
5-1. 追加コミットで積む vs squashする
| 方式 | メリット | デメリット | 使う場面 |
|---|---|---|---|
| 追加コミットで積む | レビュアーが「前回からの差分」だけ見られる | 履歴に fix review comment が残る |
レビュー往復中はこれが基本 |
| squash して1コミットに畳む | 履歴が綺麗 | レビュアーが全部を見直す羽目になる | 承認後 or マージ時(Squash mergeに任せる) |
原則:レビュー中は積む。畳むのは承認後(またはマージ時のSquash mergeに任せる)。
レビュー中にいきなり全部squashしてforce pushすると、レビュアーは「どこが変わったのか」を自力で探す必要があり、確実に嫌がられます。
5-2. --fixup + --autosquash:積みながら、綺麗に畳む
「レビュー中は差分を見せたい。でも最終的な履歴は綺麗にしたい」を両立するのが --fixup です。
どのコミットへの修正かを、コミットの時点で宣言しておく のがポイントです。
実演用のブランチはこうなっています。
git log --oneline origin/main..HEAD
abe8eb3 test: 苗字検索のテストを追加
f127937 feat: 飼い主の苗字検索コントローラを追加
ここに2件の指摘が来ました。
- 指摘1:「
lastNameが空文字のときに全件返ってしまう」→f127937の修正 - 指摘2:「このテスト、実質何も検証していない」→
abe8eb3の修正
それぞれ、対象コミットを指定して --fixup でコミットします。
# 1件目の修正をステージした状態で
git commit --fixup f127937
# 2件目の修正をステージした状態で
git commit --fixup abe8eb3
git log --oneline origin/main..HEAD
3d99d80 fixup! test: 苗字検索のテストを追加
655bb71 fixup! feat: 飼い主の苗字検索コントローラを追加
abe8eb3 test: 苗字検索のテストを追加
f127937 feat: 飼い主の苗字検索コントローラを追加
コミットメッセージが自動で fixup! <対象のメッセージ> になりました。この状態でpushすればレビュアーは追加分だけ見られます。
そして承認後、1コマンドで畳みます。
git rebase -i --autosquash origin/main
エディタに出てくるTODOリストは、すでに並べ替えられています(実際の出力)。
pick f127937 # feat: 飼い主の苗字検索コントローラを追加
fixup 655bb71 # fixup! feat: 飼い主の苗字検索コントローラを追加
pick abe8eb3 # test: 苗字検索のテストを追加
fixup 3d99d80 # fixup! test: 苗字検索のテストを追加
(ハッシュとメッセージの間の # は環境によって付かず、pick f127937 feat: ... と表示されることがあります。行の意味は同じです)
そのまま保存して閉じるだけです。
Successfully rebased and updated refs/heads/feature/owner-search.
git log --oneline origin/main..HEAD
16ae05d test: 苗字検索のテストを追加
072cf50 feat: 飼い主の苗字検索コントローラを追加
元の2コミットのまま、中身だけが修正された状態 になりました。fix review のようなノイズコミットは1つも残りません。
毎回オプションを打つのが面倒なら設定してしまう。
git config --global rebase.autosquash true
以後 git rebase -i origin/main だけで自動整列します。
コミットメッセージごと直したいとき は amend を付けます(--fixup=amend: と --fixup=reword: は git 2.32 以降 で使えます)。
git commit --fixup=amend:16ae05d
TODOリストでは fixup -C として並び、新しいコミットメッセージが採用されます(実際の出力)。
pick 072cf50 # feat: 飼い主の苗字検索コントローラを追加
pick 16ae05d # test: 苗字検索のテストを追加
fixup -C 9ac8379 # amend! test: 苗字検索のテストを追加
pick 35b8de6 # Apply suggestion: 上限件数を定数化
5-3. git range-diff:「rebase前後で何が変わったか」を見せる
履歴を書き換えると、レビュアーは「本当に指摘した箇所だけが変わったのか?」を確認できません。その不安を消すのが git range-diff です。
# 書き換え前のブランチを退避しておく(rebase前に実行)
git branch pre-squash
# rebase後に、書き換え前後を比較する
git range-diff origin/main pre-squash HEAD
実際の出力(抜粋):
1: f127937 ! 1: 072cf50 feat: 飼い主の苗字検索コントローラを追加
@@ src/main/java/com/example/owner/OwnerSearchController.java (new)
+ public List<Owner> search(String lastName) {
++ if (lastName == null || lastName.isBlank()) {
++ return List.of();
++ }
+ return repository.findByLastNameStartingWith(lastName);
2: abe8eb3 ! 2: 16ae05d test: 苗字検索のテストを追加
@@ src/test/java/com/example/owner/OwnerSearchControllerTest.java (new)
+ void search_returnsMatchedOwners() {
-+ assertThat(true).isTrue();
++ OwnerSearchController controller = new OwnerSearchController(new StubRepository());
++ assertThat(controller.search("Franklin")).hasSize(1);
3: 655bb71 < -: ------- fixup! feat: 飼い主の苗字検索コントローラを追加
4: 3d99d80 < -: ------- fixup! test: 苗字検索のテストを追加
左が書き換え前、右が書き換え後のコミット。! は「対応するが中身が変わった」、< は「消えた(=fixupに吸収された)」という意味です。この出力をPRコメントに貼れば、レビュアーは差分の確認を数秒で終えられます。
git range-diff の3引数形式は <base> <旧先頭> <新先頭> です。旧先頭を指すブランチかコミットハッシュが必要なので、rebaseの前に git branch pre-squash などで退避 しておいてください(忘れても git reflog から拾えます。後述)。
5-4. --force-with-lease を使う理由 ― --force との違いを実証する
履歴を書き換えたらforce pushが必要です。ここで --force を使ってはいけない 理由を、実際に壊して確認します。
状況:あなたがrebaseしている間に、レビュアーがGitHub上で「Commit suggestion」を押しました。suggestionを確定すると、あなたのブランチに直接コミットが積まれます。 あなたはそれをまだfetchしていません。
まず普通のpushを試すと、当然ながら拒否されます。
git push origin feature/owner-search
To /path/to/origin.git
! [rejected] feature/owner-search -> feature/owner-search (fetch first)
error: failed to push some refs to '/path/to/origin.git'
hint: Updates were rejected because the remote contains work that you do not
hint: have locally. This is usually caused by another repository pushing to
hint: the same ref. If you want to integrate the remote changes, use
hint: 'git pull' before pushing again.
ここで --force-with-lease を使うと、ちゃんと止めてくれます。
git push --force-with-lease origin feature/owner-search
To /path/to/origin.git
! [rejected] feature/owner-search -> feature/owner-search (stale info)
error: failed to push some refs to '/path/to/origin.git'
stale info =「お前が知っているリモートの状態は古い」という意味です。他人のpushを検知して弾いてくれた わけです。
一方、--force を使うとどうなるか。同じ状況で実行してみます。
# push前のリモートの状態
git -C /path/to/origin.git log --oneline -3 feature/owner-search
cd48d8a Apply suggestion: 上限件数を定数化
abe8eb3 test: 苗字検索のテストを追加
f127937 feat: 飼い主の苗字検索コントローラを追加
git push --force origin feature/owner-search
To /path/to/origin.git
+ cd48d8a...16ae05d feature/owner-search -> feature/owner-search (forced update)
# push後のリモートの状態
git -C /path/to/origin.git log --oneline -3 feature/owner-search
16ae05d test: 苗字検索のテストを追加
072cf50 feat: 飼い主の苗字検索コントローラを追加
554fcba chore: プロジェクト初期化
レビュアーのコミット cd48d8a が、警告ひとつなく消えました。
しかも消したことに気づけません。レビュアーが「さっき提案を反映したはずなのに戻ってる」と言い出して、ようやく発覚します。これが --force の怖さです。
git push --force はエイリアスにすら書かない。 --force-with-lease を常用してください。エイリアス例:
git config --global alias.pushf 'push --force-with-lease --force-if-includes'
--force-if-includes(git 2.30以降)を併用すると、「fetchはしたが相手のコミットを自分の履歴に取り込んでいない」ケースも弾けるので、より安全です。
正しい復旧手順 はこうです。
git fetch origin
git log --oneline -3 origin/feature/owner-search # 相手が何を積んだか確認
cd48d8a Apply suggestion: 上限件数を定数化
abe8eb3 test: 苗字検索のテストを追加
f127937 feat: 飼い主の苗字検索コントローラを追加
git cherry-pick cd48d8a # 相手のコミットを自分の履歴に取り込む
git push --force-with-lease origin feature/owner-search
To /path/to/origin.git
+ cd48d8a...35b8de6 feature/owner-search -> feature/owner-search (forced update)
相手のコミットを保持したまま 更新できました。
5-5. force pushしてよいタイミング / ダメなタイミング
| 状況 | 可否 | 理由 |
|---|---|---|
| 自分専用のトピックブランチで、レビュー開始前 | ⭕ | 誰も見ていない |
| レビュー中だが、レビュアーに「履歴を整理します」と伝えてある | ⭕ | 合意がある。range-diff を添えるとなお良い |
| 承認後、マージ直前の最終整理 | ⭕ | 差分の意味は確定済み |
| レビュー中に無言で | ❌ | レビュアーが読んでいた行番号・コメント位置がズレる |
| レビュアーがsuggestionを積んだ直後 | ❌ | 相手のコミットを消す(上で実証したとおり) |
| 他人も作業している共有ブランチ | ❌❌ | 論外。develop main などは当然不可 |
やらかしても reflog があります。 rebaseやreset直後なら、元の状態に戻せます。
git reflog -6 --date=short
35b8de6 HEAD@{2026-07-30}: cherry-pick: Apply suggestion: 上限件数を定数化
16ae05d HEAD@{2026-07-30}: rebase (finish): returning to refs/heads/feature/owner-search
16ae05d HEAD@{2026-07-30}: rebase (fixup): test: 苗字検索のテストを追加
b5e4e9b HEAD@{2026-07-30}: rebase (pick): test: 苗字検索のテストを追加
072cf50 HEAD@{2026-07-30}: rebase (fixup): feat: 飼い主の苗字検索コントローラを追加
f127937 HEAD@{2026-07-30}: rebase (start): checkout origin/main
git reset --hard <戻りたいハッシュ> で復帰できます。ただしリモートを --force で壊した場合、相手のコミットは自分のreflogには無い ので、相手の手元から復旧してもらう必要があります。だから予防(--force-with-lease)が重要なのです。
6. Step 4:返信の書き方
「修正しました」だけで返さない
これが最頻出のNGです。レビュアー視点で何が困るかというと:
- どう直したのか分からない(意図どおりか確認するには、全差分を自分で読むしかない)
- どのコミットで直したのか分からない(複数コメントに全部「修正しました」と返ってくると、対応漏れの検出ができない)
返信に含めるべき要素は3つだけです。
- 何をどう直したか(1行)
- 該当コミットのハッシュ or 変更後の行へのリンク
- 判断が入った場合はその理由
良い返信テンプレ
① 指摘どおり直した場合
ご指摘ありがとうございます。空文字・null のときに全件返る問題を確認しました。
early return で空リストを返すように修正し、そのケースのテストも追加しています(072cf50)。
② 指摘とは別の方法で直した場合(重要)
ありがとうございます。原因は同じ認識です。
ただ、ご提案いただいたバリデーションアノテーションでの対応だと
このメソッドが内部からも呼ばれるためすり抜けるケースがあり、
メソッド先頭でのガードに変更しました(072cf50)。意図が異なっていればご指摘ください。
③ 別チケットに切り出す場合
キャッシュ導入、確かに効果が大きそうです。
ただ本PRの目的(検索機能の追加)からは範囲が広がるため、別チケットにさせてください。
→ #432 を作成しました。今スプリントの残りで着手予定です。
④ 質問への返信(コードは変えない)
この分岐は、〇〇の仕様でXXのケースが存在するために必要です(仕様書 3.2 / テスト XxxTest#shouldReturnEmpty で保証しています)。
コメントが無く読み取りづらかったので、意図を1行コメントとして追記しました(16ae05d)。
④が示しているとおり、「説明が必要だった」=「コードが説明不足だった」ことが多い です。質問に対しては、コメント追加やテスト名の改善で応じると評価が高くなります。
最後にまとめコメントを1つ置く
個別返信をした上で、再レビュー依頼時にサマリを1つ 投げます。
対応が完了しましたので、再レビューをお願いします。
| # | 指摘 | 対応 | コミット |
|---|------|------|---------|
| 1 | null/空文字で全件返る | 修正(early return + テスト追加) | 072cf50 |
| 2 | テストが実質検証していない | 修正(スタブでの検証に変更) | 16ae05d |
| 3 | 上限件数の定数化 | suggestion を取り込み | 35b8de6 |
| 4 | キャッシュ導入 | 別チケット #432 | — |
なお、fixupコミットを畳んで履歴を整理しています。
整理前後の差分は `git range-diff` で確認いただけます(結果は上のコメントに貼付)。
この1コメントがあるかないかで、再レビューの所要時間が変わります。 レビュアーは「4件中3件がこのPR、1件は別チケット」を即座に把握でき、確認すべき差分の位置も分かります。
7. 認識がズレている/指摘が間違っていると思ったとき
新人がいちばん困る場面です。原則はひとつです。
感情でも立場でもなく、コード・テスト・計測・仕様で返す。
「私はこう思います」は議論を長引かせます。検証可能な事実を1つ出す と、たいてい1往復で終わります。
文面テンプレ
① 前提が食い違っている疑いがあるとき(最優先で使う)
確認させてください。ご指摘は「XはYのとき必ず非nullになる」という前提でしょうか。
私はZのケースでnullになり得ると理解しており(AaaService#find の 120行目で null を返す経路があります)、
そのため現在のガードを入れています。前提の認識が合っているか教えていただけますか。
② 指摘が誤りだと考えられるとき(テストで示す)
検証してみました。ご懸念のケースを再現するテストを追加したところ、
現行実装でも期待どおりに動作していました(BbbTest#shouldHandleEmptyInput / 16ae05d)。
私の再現方法が間違っていたら教えてください。
③ 性能・効率が論点のとき(数字で示す)
パフォーマンスの観点、ありがとうございます。
手元で1万件のデータで計測したところ、変更前 820ms / 変更後 790ms でした。
差が小さいため、可読性を優先して現在の実装のままにしたいと考えています。
測定条件が不適切であればご指摘ください。
④ どちらも正しく、決めきれないとき(判断を委ねる)
どちらの書き方でも要件は満たせると考えています。
- 案A(現状): 〇〇。ただし△△
- 案B(ご提案): △△が解消。ただし〇〇
チームの規約に沿うほうに合わせたいので、Aで進めてよいか、Bに変更すべきか判断いただけますか。
型は「根拠 → 代案 → 判断を委ねる」
4つのテンプレに共通する構造です。
- 根拠:コード位置・テスト・数値・仕様書のいずれか(必ず検証可能な形で)
- 代案:「直さない」で終わらせず、別の解決策か現状維持の理由を出す
- 判断を委ねる:「間違っていたら教えてください」で締める
3つ目が特に大事です。勝ち負けにしない。 目的は正しいコードをマージすることであって、議論に勝つことではありません。
それでも折り合わないときは、テキストでの往復を3回までにする。 4往復目に入りそうなら「口頭で5分いただけますか」に切り替えてください。テキストの往復は認識差を埋めるのが遅く、感情的なすれ違いも生みやすいです。
8. GitHub上の実務テク
8-1. suggestion(提案コミット)の受け方
レビュアーが suggestion 記法で書いたコメントは、GitHub上のボタンでそのままコミットできます。
| 操作 | 挙動 | 注意 |
|---|---|---|
| Commit suggestion | その場で1コミット追加される |
あなたのブランチにコミットが積まれる。手元は古くなるので必ず git pull / git fetch
|
| Add suggestion to batch | 複数の提案をまとめて1コミットに | 体裁系の提案が大量にあるときに有効 |
| 手元で直す | 自分でコミットを作る | 「suggestionを取り込みました」と返信する。resolveだけして無視しない |
最大の事故は、suggestionをコミットしてもらった直後に、手元の古い履歴を force push して消してしまうこと です(第5章で実証したケースそのもの)。suggestionが使われる現場では、pushの前に必ず git fetch する癖をつけてください。
8-2. CLIでのレビュー対応
ブラウザとCLIを行き来すると見落とします。gh でまとめて処理できます。
gh pr view --comments # コメント全部
gh pr diff # 差分(レビュアーが見ているものと同じ)
gh pr diff --name-only # 変更ファイル一覧
gh pr checks # CI状態
gh pr comment --body "..." # まとめコメントを投稿
gh pr edit --add-reviewer <user> # 再レビュー依頼
gh pr ready # Draft を Ready に切り替え
gh pr diff --name-only の実行例(spring-petclinic の実在PR):
src/main/java/org/springframework/samples/petclinic/owner/PetController.java
src/main/resources/db/h2/schema.sql
src/main/resources/db/mysql/schema.sql
src/main/resources/db/postgres/schema.sql
src/test/java/org/springframework/samples/petclinic/PetClinicConcurrencyTests.java
src/test/java/org/springframework/samples/petclinic/owner/PetControllerTests.java
src/test/java/org/springframework/samples/petclinic/service/ClinicServiceTests.java
PRの規模を数値で確認したいときは JSON で取れます。
gh pr view 2573 -R spring-projects/spring-petclinic \
--json number,title,changedFiles,additions,deletions \
--jq '{number,title,changedFiles,additions,deletions}'
{"additions":266,"changedFiles":7,"deletions":6,"number":2573,"title":"Prevent duplicate pet creation with similar name for the same owner under concurrent requests"}
8-3. レビュー中に差分が膨らんだときは、分割する
指摘対応をしていたら変更が2倍になった、はよくあります。そのまま進めると、レビュアーの負荷が跳ね上がって承認が遠のきます。
判断基準はシンプルです。
| 状況 | 対応 |
|---|---|
| 指摘対応の結果、変更行数が当初の2倍を超えた | 分割を検討する |
| 「ついでにリファクタ」が混ざった | 確実に分割する(レビュアーが機能変更とリファクタを区別できない) |
| 別チケット相当の改善が入り込んだ | 切り出して別PRにする |
分割の実務手順(現在のブランチから、特定コミットだけを別ブランチに移す例):
# 1. マージしたい分だけの新しいブランチを、mainから作る
git switch -c feature/owner-search-core origin/main
# 2. 必要なコミットだけ持ってくる
git cherry-pick 072cf50 16ae05d
# 3. push して、小さいPRを先に出す
git push -u origin feature/owner-search-core
「大きい1つ」より「小さい2つ」のほうが、合計のマージ完了は確実に早い です。レビュアーの認知負荷は行数に対して線形ではなく、指数的に増えます。
9. 同じ指摘を二度もらわない仕組み化
ここが、レビュー対応の本当の成果です。
同じ指摘を2回もらったら、それは自分の注意不足ではなく 仕組みの不在 だと考える。
指摘 → 仕組みの変換表
| 指摘の種類 | 落とし込み先 | 具体例 |
|---|---|---|
| インデント・import順・空行 | formatter |
spring-javaformat / spotless / prettier / gofmt
|
| 未使用変数・命名規約・禁止API | linter / 静的解析 | Checkstyle / SpotBugs / ESLint / SwiftLint |
| テスト漏れ・カバレッジ低下 | CI |
mvn verify を必須チェック化、カバレッジ閾値 |
| 「動作確認した?」「影響範囲は?」 | PRテンプレート | .github/pull_request_template.md |
| 設計方針・命名の流儀 | ドキュメント + AIの設定ファイル |
CONTRIBUTING.md / CLAUDE.md
|
| 自分だけがやりがちなミス | 自分用チェックリスト | 手元のメモ。PR出す前に3分で見る |
実例:ビルドで体裁を強制する
spring-petclinic は、フォーマット違反を ビルドの validate フェーズで落とす 設定になっています(実際の pom.xml より抜粋)。
<plugin>
<groupId>io.spring.javaformat</groupId>
<artifactId>spring-javaformat-maven-plugin</artifactId>
<version>${spring-format.version}</version>
<executions>
<execution>
<goals>
<goal>validate</goal>
</goals>
<phase>validate</phase>
</execution>
</executions>
</plugin>
そしてCIでは、単に verify を叩くだけです(.github/workflows/maven-build.yml より抜粋)。
- name: Build with Maven Wrapper
run: ./mvnw -B verify
この2つがあると、体裁の指摘はそもそもレビューに到達しません。 人間は設計とロジックだけを見ればよくなります。
上記はいずれも既存プロジェクトの設定ファイルからの抜粋です(${spring-format.version} などのプロパティは同ファイル内で定義されています)。自分のプロジェクトに入れる際は、バージョン定義とセットで導入してください。
実例:PRテンプレートで指摘を先回りする
.github/pull_request_template.md を置くと、PR作成時に本文へ自動展開されます。
## 目的
<!-- なぜこの変更が必要か。チケットURL -->
## 変更内容
<!-- 何をどう変えたか。設計判断があれば理由も -->
## 動作確認
- [ ] ローカルで正常系を確認した
- [ ] 異常系(null / 空 / 上限)を確認した
- [ ] 既存テストが通ることを確認した
## 影響範囲
<!-- 触ったメソッドの呼び出し元、DBスキーマ変更の有無 -->
## レビューしてほしい観点
<!-- 特に見てほしい箇所。迷った箇所 -->
最後の「レビューしてほしい観点」が効きます。迷った箇所を自分から開示すると、レビュアーはそこを重点的に見てくれる ので、的外れな往復が減ります。
実例:AIの設定ファイルに落とす
Claude Code を使っているなら、指摘内容を CLAUDE.md に書いておくと、次回以降の生成コードが最初からその規約に従います。
## コードレビューで繰り返し指摘されたルール
- publicメソッドの引数は先頭でnull/空文字をガードする(後段でNPEにしない)
- テストは `assertThat(true).isTrue()` のような無意味なアサーションを書かない
- 1メソッド1責務。分岐が3階層を超えたらメソッド抽出を検討する
- ログは `System.out.println` ではなく Logger を使う
「人の注意力に頼るのをやめて、仕組みに移す」 ── これができる人は、レビュアーから見て「学習する人」になります。同じ指摘を3回もらう人と、1回で仕組み化する人の差は、半年で決定的になります。
10. Claude Code に指摘対応を任せるときの線引き
レビュー対応はAIと相性が良い作業ですが、任せていい領域と絶対に任せてはいけない領域 がはっきり分かれます。
| 作業 | 任せる | 理由 |
|---|---|---|
| 体裁・命名の一括修正 | ⭕ | 機械的。差分も検証しやすい |
| 同じパターンの修正を全ファイルに展開 | ⭕ | 人間がやると漏れる |
| テストケース追加のたたき台作成 | ⭕ | ただしアサーションの妥当性は自分で判断 |
| 指摘内容の整理・分類のたたき台 | ⭕ | 最終判断は自分 |
| 設計をどう変えるかの判断 | ❌ | 現場の制約・過去の経緯を知らない |
| 「直さない」判断とその根拠づくり | ❌ | 責任を持てるのは自分だけ |
| レビュアーへの返信文の丸投げ | ❌ | 下記参照 |
返信文をAIに丸投げしてはいけない理由
- 事実確認をしていない断定が混ざる(「テストで確認済みです」など、確認していないのに書かれる)
- 不自然に丁寧すぎる長文になり、読む側の時間を奪う
- 何より、書いた本人が内容を理解していないことがレビュアーに伝わる
たたき台を出させるのは構いません。ただし 送信前に、書かれている事実がすべて検証済みか自分で確認する こと。「テストを追加しました」と書くなら、そのテストが実際に通っていることを自分で確認してから送る。ここは絶対に省略できません。
AIに直させた差分は、自分でレビューし直す
これは義務だと思ってください。レビュー指摘の修正をAIに任せて、その差分を読まずにpushするのは、レビュアーへの裏切りです。
# 1. 変更の全体像を掴む
git diff --stat
# 2. 1ハンク(変更のまとまり)ずつ確認しながらステージする
git add -p
# 3. テストを実行する(AIの修正でテストが「通るように」書き換えられていないかも見る)
./mvnw -B verify
# 4. 意図しない変更が混ざっていないか、最終確認
git diff --cached
特に git add -p は必須です。「指摘と無関係なファイルが書き換わっている」「テストのアサーションが緩められている」といったAI特有の余計な変更を、ここで確実に止められます。
(AIをレビューする側として使う話は、Claude Code のサブエージェント機能でコードレビュー職人を作った話 に書きました。この記事は受ける側に徹しています)
11. やってはいけないこと8つ
| # | NG行動 | なぜダメか | 代わりにやること |
|---|---|---|---|
| 1 | 黙って直して黙ってpushする | レビュアーは全差分を読み直す羽目になる。対応漏れも検出できない | 各コメントに返信+まとめコメント |
| 2 | レビュー中に全部squashしてforce push | 前回からの差分が消え、レビューが振り出しに戻る | レビュー中は積む。畳むなら range-diff を添える |
| 3 |
git push --force を使う |
相手のコミットを無言で消す(第5章で実証) |
--force-with-lease(+--force-if-includes) |
| 4 | 「すみません」だけ連投する | 情報がゼロ。読む時間だけ奪う | 謝罪は1回。以降は事実と対応内容 |
| 5 | 指摘を人格否定と受け取る | 思考が止まり、質問もできなくなる | 指摘はコードに対するもの。分類という作業に落として機械的に処理する |
| 6 | 納得できないまま「直しました」と言う | 同じ設計ミスを次のPRでも再生産する | 理解できるまで質問する(質問は評価を下げない) |
| 7 | レビュアーを待たせたまま放置する | ブランチが腐り、コンフリクトが増える | 時間がかかるなら「◯日までに対応します」と先に伝える |
| 8 | 指摘対応のついでに無関係なリファクタを混ぜる | 差分が膨らみ、レビュー不能になる | 別PRに切り出す(第8章の分割手順) |
5番が本質です。 指摘を感情の問題にすると手が止まります。「分類する → 順序を決める → 直す → 返信する」という作業に落とし込めば、感情が入る余地がなくなります。この記事全体が、そのための手順書です。
12. チェックリスト(コピペして使う)
受領直後
- 1つも直さずに 全コメントを最後まで読んだ
-
gh pr view --commentsで見落としがないか確認した - 5分類(必須/推奨/好み/質問/認識ズレ)に振り分けた
- 認識ズレの疑いがあるものは、着手前に質問した
- 別チケット化するものは、実際にチケットを作ってURLを控えた
- 方針表をコメントとして投稿した
修正中
- 設計 → ロジック → 命名 → 体裁 の順で直している
-
修正ごとに
git commit --fixup <対象コミット>で積んでいる - 体裁はformatterで一括処理した(手作業していない)
-
AIに直させた差分を
git add -pで1ハンクずつ確認した - テストを追加/更新し、ローカルで通した
push前
-
git fetchして、相手のsuggestionコミットが無いか確認した -
履歴を書き換えたなら
git branch pre-squashで退避してある -
push は
--force-with-lease(--forceを使っていない) -
gh pr checksでCIが緑になった
再レビュー依頼
- すべてのコメントに個別返信した(「修正しました」だけになっていない)
- 各返信にコミットハッシュ or 行リンクを入れた
- 対応しなかったものは、根拠・代案・判断委ねの3点セットで返した
- まとめコメント(対応表)を投稿した
-
履歴を整理した場合、
git range-diffの結果を貼った -
レビュアーに再レビューを依頼した(
gh pr edit --add-reviewer等)
事後(ここまでやって完了)
- 同じ指摘を防ぐ仕組み(formatter / lint / CI / PRテンプレ / CLAUDE.md)に落とした
- 自分用チェックリストを1行更新した
まとめ
- まず全部読む。1つも直さない。 上から順に処理するのは優先度の放棄
- コメントは 必須/推奨/好み/質問/認識ズレ の5分類。「直す」以外に 別チケット化・議論・現状維持 という正当な選択肢がある
- 別チケット化するなら その場でチケットを作ってURLを貼る
- 修正は 設計 → ロジック → 命名 → 体裁。逆順は全部やり直しになる
- レビュー中は
--fixupで積む、承認後に--autosquashで畳む。整理したらrange-diffを見せる -
--forceは使わない。--force-with-lease。 実際に相手のコミットが消えることを本文で実証した - 返信は 何をどう直したか+コミットハッシュ+理由。「修正しました」だけで返さない
- 納得できない指摘には 根拠 → 代案 → 判断を委ねる の型で返す。勝ち負けにしない
- 同じ指摘を2回もらったら 仕組みの不在。formatter / CI / PRテンプレ / CLAUDE.md に落とす
- AIには機械的な修正だけ任せる。設計判断と返信文は自分。AIの差分は
git add -pで自分がレビューする
レビューは「品質チェック」であると同時に、あなたがどう働く人かを見られている場 でもあります。同じ内容の修正でも、手順が整っている人は次から大きな仕事を任されます。
参考
- git-commit Documentation(
--fixup/--fixup=amend:) - git-rebase Documentation(
--autosquash) - git-push Documentation(
--force-with-lease/--force-if-includes) - git-range-diff Documentation
- GitHub Docs: Incorporating feedback in your pull request(suggestionの適用)
- GitHub Docs: Creating a pull request template for your repository
- gh pr view(GitHub CLI Manual)
- spring-projects/spring-petclinic(本記事で参照した実在リポジトリ)
エンジニア採用や開発のご相談は、コーポレートサイトからお気軽にどうぞ。
▶株式会社Good Lab コーポレートサイト
@kotaro_ai_lab
AI活用や開発効率化について発信しています。フォローお気軽にどうぞ!