pr-quality-checklist

PR 品質チェックリストスキル

概要

レビューしやすく、十分に文書化され、チームの基準に従った高品質なプルリクエストを作成するための包括的なガイドです。作成者とレビュアーの両方を対象に、テンプレート、サイズのガイドライン、ベストプラクティスを紹介します。

使用するタイミング

• プルリクエストを作成する前
• 他の人の PR をレビューするとき
• チームの PR 基準を定めるとき
• 新しいチームメンバーのオンボーディング時
• PR のレビューに時間がかかりすぎているとき

クイックチェックリスト

PR 作成前

• PR のタイトルが規約に従っている：type(scope): description
• 説明に概要、関連チケット、テスト計画が含まれている
• 変更の範囲が絞られ、関連性がある（単一の懸念事項）
• 明らかな問題がないかコードをセルフレビューした
• テストを追加・更新し、すべて成功している
• デバッグ用コード（console.log、debugger など）がない
• TypeScript／型のエラーを解消した
• 必要に応じてドキュメントを更新した
• UI の変更にはスクリーンショットを含めた
• Changeset を追加した（ユーザー向けの変更の場合）

---

PR タイトルの形式

規約

{type}({scope}): {short description}

種類

• feat：新機能
• fix：バグ修正
• refactor：バグ修正や機能追加を伴わないコード変更
• docs：ドキュメントのみの変更
• style：フォーマット、セミコロンの不足など（コードの変更なし）
• test：テストの追加または更新
• chore：メンテナンスタスク、依存関係の更新
• perf：パフォーマンスの改善
• ci：CI／CD の変更

例

feat(auth): add OAuth2 login support
fix(cart): resolve race condition in checkout
refactor(search): extract query param parsing logic
docs(api): update authentication examples
test(payment): add integration tests for Stripe
chore(deps): update Next.js to v15
perf(images): implement lazy loading for gallery
ci(deploy): add staging environment workflow

スコープのガイドライン

• コンポーネント／モジュール名を使用します：auth、cart、search、api
• 具体的にしつつ、細かくしすぎないようにします：user-profile-settings-modal ではなく user-profile
• コードベースの構成に合わせます

---

PR 説明のテンプレート

標準テンプレート

## Summary

- Added OAuth2 authentication flow
- Integrated with Auth0 provider
- Updated login UI components

## Related Tickets

- Closes #123
- Related to #456
- Fixes ENG-789

## Changes

### Added
- `src/lib/auth/oauth2.ts` - OAuth2 client implementation
- `src/app/api/auth/callback/route.ts` - Auth callback handler
- `src/components/OAuthButtons.tsx` - OAuth login buttons

### Modified
- `src/components/LoginForm.tsx` - Added OAuth buttons
- `src/lib/auth/index.ts` - Export new auth methods
- `README.md` - Updated setup instructions

### Removed
- `src/lib/auth/legacy.ts` - Deprecated auth code
- `src/components/OldLoginForm.tsx` - Replaced by new form

## Screenshots

### Desktop
![Desktop view](https://imgur.com/desktop.png)

### Mobile
![Mobile view](https://imgur.com/mobile.png)

### Before/After (if applicable)
| Before | After |
|--------|-------|
| ![Before](before.png) | ![After](after.png) |

## Testing

- [ ] Unit tests added/updated
- [ ] Integration tests pass
- [ ] Manual testing completed
- [ ] Tested on Chrome, Firefox, Safari
- [ ] Mobile responsive (tested on iOS/Android)
- [ ] Edge cases tested (empty state, error states, loading)

## Breaking Changes

⚠️ **Breaking Change**: The `login()` function signature has changed.

**Before:**
```typescript
login(username: string, password: string)

変更後：

login({ username: string, password: string, provider?: 'local' | 'oauth' })

移行：

// Old
await login('user', 'pass');

// New
await login({ username: 'user', password: 'pass' });

チェックリスト

• コードがプロジェクトのスタイルガイドラインに従っている
• セルフレビューを完了した
• 複雑なロジックにコメントを追加した
• 必要に応じてドキュメントを更新した
• Changeset を追加した（ユーザー向けの変更の場合）
• console.log／debugger の記述が残っていない
• TypeScript のエラーがない
• ローカルですべてのテストが成功する
• ビルドが成功する

### Minimal Template (for small changes)
```markdown
## Summary
Brief description of the change.

## Changes
- Modified `file.ts` to fix XYZ

## Testing
- [ ] Tests pass
- [ ] Verified manually

---

サイズのガイドライン

理想的な PR のサイズ

• 変更行数：300 行未満（生成ファイルを除く）
• 変更ファイル数：15 未満
• レビュー時間：30 分未満
• コミット数：論理的にまとまったコミットが 1～5 件

サイズの分類

サイズ	行数	ファイル数	レビュー時間	推奨事項
XS	< 50	1-3	5 min	✅ 緊急修正に最適
S	50-150	3-8	15 min	✅ 適切なサイズ
M	150-300	8-15	30 min	⚠️ 分割を検討
L	300-500	15-25	1 hour	❌ 分割すべき
XL	500+	25+	2+ hours	❌ 分割必須

PR を分割するタイミング

1. 機能のフェーズごと

PR #1: MVP implementation (core functionality)
PR #2: Polish and edge cases
PR #3: Additional features

2. レイヤーごと

PR #1: Database schema changes
PR #2: Backend API implementation
PR #3: Frontend UI integration
PR #4: Tests and documentation

3. 懸念事項ごと

PR #1: Refactoring (no behavior change)
PR #2: New feature (builds on refactored code)
PR #3: Tests for new feature

4. リスクレベルごと

PR #1: High-risk changes (need careful review)
PR #2: Low-risk changes (routine updates)

サイズ制限の例外

• 生成コード（マイグレーション、API クライアント、型）
• ファイルの名前変更／移動（git mv を使って示します）
• 一括フォーマット変更（別の PR にし、事前承認を得ます）
• サードパーティライブラリの統合（十分に文書化されていること）

---

リファクタリング PR

リファクタリング用テンプレート

## Summary
Refactoring and cleanup for [area]

**Goal**: Improve code maintainability without changing behavior

## Motivation
- Reduce code duplication (3 similar functions → 1 reusable utility)
- Improve type safety (any → specific types)
- Remove dead code identified during feature work

## Related Tickets
- ENG-XXX: Improve [area] maintainability
- ENG-YYY: Remove deprecated [feature]

## Stats
- **Lines added**: +91
- **Lines removed**: -1,330
- **Net**: -1,239 ✅
- **Files changed**: 23

## Changes

### Removed (Dead Code)
- `/api/old-endpoint` - unused, replaced by `/api/new-endpoint` in v2.0
- `useDeprecatedHook.ts` - replaced by `useNewHook.ts` (ENG-234)
- `legacy-utils.ts` - functions no longer called anywhere

### Refactored
- **Extracted common logic**: Query param parsing now in `lib/url-utils.ts`
- **Consolidated validation**: 3 duplicate Zod schemas → 1 shared schema
- **Improved types**: Replaced 12 `any` types with proper interfaces

### No Behavior Changes
- [ ] All tests pass (no test modifications needed)
- [ ] Same inputs → same outputs
- [ ] No user-facing changes

## Testing
- [ ] Full test suite passes (no new tests needed)
- [ ] Manual smoke test completed
- [ ] No regressions identified

## Review Focus
- Verify removed code is truly unused (checked with `rg` search)
- Confirm refactored logic is equivalent
- Check no subtle behavior changes introduced

リファクタリングのベストプラクティス

• リファクタリングと機能を分ける：決して混在させないでください
• 動作に変更がないことを確認する：テストの更新が不要であること
• 削除したコードを説明する：安全に削除できる理由を説明します
• 十分に検索する：rg／grep を使って使用箇所がないことを確認します
• LOC の削減をアピールする：コードの削減を強調します

---

レビューチェックリスト

作成者向け（セルフレビュー）

レビューを依頼する前に、以下を確認してください。

コード品質

• コードが DRY である（明らかな重複がない）
• 関数の目的がそれぞれ単一で、焦点が絞られている
• 変数名が明確で、内容を適切に表している
• マジックナンバーやハードコードされた値がない
• エラー処理が包括的である
• エッジケースに対応している

テスト

• 新しい機能をテストでカバーしている
• テストに意味がある（カバレッジのためだけではない）
• エッジケースをテストしている
• エラー条件をテストしている
• 不安定なテストを新たに導入していない

ドキュメント

• 複雑なロジックにはコメントがある
• 公開 API に JSDoc／docstring がある
• README が更新されている（セットアップに変更があった場合）
• 移行ガイドがある（互換性を損なう変更がある場合）

パフォーマンス

• 明らかなパフォーマンス上の問題がない
• データベースクエリが効率的である（インデックスを考慮している）
• N+1 クエリがない
• 大規模なデータセットが適切に処理される

セキュリティ

• ハードコードされたシークレットがない
• 入力検証が実装されている
• 認可チェックが実装されている
• SQL インジェクションの脆弱性がない
• 機密データがログに記録されない

---

レビュアー向け

最初の確認（5 分）

• PR の説明が明確である
• 変更内容が明記された目的に沿っている
• 規模が妥当である（300 LOC 未満が望ましい）
• 明らかな懸念事項がない

コードレビュー（15～30 分）

• ロジックが正しい
• エッジケースに対応している
• エラーハンドリングが適切である
• コードが読みやすく、保守しやすい
• パフォーマンス上の問題がない
• セキュリティ上の考慮事項に対処している

テストの確認

• テストが変更内容を網羅している
• テストに意味がある
• 不安定なテストがない
• エッジケースをテストしている

ドキュメントの確認

• PR の説明が変更内容と一致している
• コメントが「何を」ではなく「なぜ」を説明している
• 互換性を損なう変更が文書化されている
• 必要に応じて移行ガイドが用意されている

承認基準

✅ Approve: Code is good, minor nits only
💬 Comment: Suggestions but not blockers
🔄 Request Changes: Issues must be fixed before merge

---

よくある間違い

1. 曖昧な PR タイトル

❌ "Fix bug"
❌ "Updates"
❌ "WIP"
❌ "Changes from code review"

✅ "fix(auth): prevent duplicate login requests"
✅ "feat(cart): add coupon code support"

2. コンテキストの不足

❌ "Changed the function"
Why? What was wrong? What's the impact?

✅ "Refactored parseQueryParams to handle nested objects

Previously failed when query params contained dots (e.g., user.name).
Now correctly parses nested structures using qs library.

Fixes ENG-456"

3. 変更規模が大きすぎる

❌ 2,000 lines changed across 50 files
❌ "Implement entire authentication system"

✅ Split into:
  - PR #1: Add database schema for auth (150 LOC)
  - PR #2: Implement JWT utilities (100 LOC)
  - PR #3: Create login endpoint (200 LOC)
  - PR #4: Add login UI (250 LOC)

4. 異なる関心事の混在

❌ Single PR with:
  - Feature implementation
  - Dependency updates
  - Refactoring
  - Bug fixes
  - Formatting changes

✅ Separate PRs:
  - PR #1: feat(feature)
  - PR #2: chore(deps)
  - PR #3: refactor(component)

5. UI 変更のスクリーンショットがない

❌ "Updated the header design"
(No screenshots)

✅ "Updated the header design"
[Desktop screenshot]
[Mobile screenshot]
[Before/After comparison]

6. テストに関する説明が不十分

❌ "Tested locally"
How? What scenarios? What browsers?

✅ "Testing completed:
- Unit tests: All 47 tests pass
- Manual testing: Tested login flow on Chrome, Firefox, Safari
- Edge cases: Tested expired tokens, invalid credentials, network errors
- Mobile: Verified on iOS Safari and Android Chrome"

7. デバッグコードの残存

❌ console.log('user data:', user);
❌ debugger;
❌ // TODO: fix this later
❌ import { test } from './test-utils'; (unused)

✅ Clean code without debugging artifacts

8. Changeset がない（ユーザー向けの変更の場合）

❌ New feature shipped without changelog entry

✅ .changeset/new-feature.md:
---
"@myapp/web": minor
---

Add OAuth2 login support with Auth0 integration

---

まとめ

クイックリファレンス

PR タイトルの形式

{type}({scope}): {clear, concise description}

理想的な PR の特徴

• 規模：300 LOC 未満、15 ファイル未満
• 焦点：単一の関心事に絞り、関連する変更のみを含む
• ドキュメント：明確な説明、テスト計画、スクリーンショット（UI の場合）
• 品質：セルフレビュー済み、テスト済み、デバッグコードなし
• タイミング：マージ可能な状態であり、WIP ではない

レビューの心構え

作成者として：

• レビュアーが作業しやすいようにする
• コンテキストと理由を示す
• レビューを依頼する前に十分にテストする
• フィードバックに迅速に対応する

レビュアーとして：

• 建設的かつ具体的に伝える
• 正確性と保守性を重視する
• 不明な点は質問する
• 優れたプラクティスを評価する

懸念事項

• 🚩 説明がない、またはタイトルが「WIP」
• 🚩 変更が 500 LOC を超えている
• 🚩 異なる関心事が混在している（機能追加＋リファクタリング＋依存関係の変更）
• 🚩 テストがない、またはテストだけを変更している
• 🚩 「動くので信じてください」
• 🚩 Console.log／debugger 文が残っている
• 🚩 TypeScript のエラーを無視している
• 🚩 UI 変更のスクリーンショットがない

このスキルを使って、PR の高い品質基準を維持し、コードレビューのプロセスを効率化しましょう。