0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

コードレビューで指摘をもらった後、何をすべきか ― レビュー対応で信頼を得る人の手順

0
Posted at

株式会社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)

逆順の悲劇

④→①でやると、こうなります。

  1. import順を整え、typoを直し、変数名を丁寧にリネームする(30分)
  2. その後、①の「このクラスはServiceに移すべき」に着手する
  3. クラスごと移動 → 直したはずの命名も体裁も全部やり直し
  4. さらに、移動後のクラスに対してレビュアーから体裁の再指摘

①が確定していないうちに②〜④をやるのは、土台が決まっていない家の壁紙を貼る ようなものです。

逆に、④は最後に一括で潰せる

体裁はほぼ機械で直せます(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. 何をどう直したか(1行)
  2. 該当コミットのハッシュ or 変更後の行へのリンク
  3. 判断が入った場合はその理由

良い返信テンプレ

① 指摘どおり直した場合

ご指摘ありがとうございます。空文字・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つのテンプレに共通する構造です。

  1. 根拠:コード位置・テスト・数値・仕様書のいずれか(必ず検証可能な形で)
  2. 代案:「直さない」で終わらせず、別の解決策か現状維持の理由を出す
  3. 判断を委ねる:「間違っていたら教えてください」で締める

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 で自分がレビューする

レビューは「品質チェック」であると同時に、あなたがどう働く人かを見られている場 でもあります。同じ内容の修正でも、手順が整っている人は次から大きな仕事を任されます。


参考


エンジニア採用や開発のご相談は、コーポレートサイトからお気軽にどうぞ。
▶株式会社Good Lab コーポレートサイト

@kotaro_ai_lab
AI活用や開発効率化について発信しています。フォローお気軽にどうぞ!

0
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?