Loading...
Loading...
PR quality checklist for ensuring comprehensive, well-documented pull requests. Use before submitting PRs to improve review efficiency and code quality.
npx skill4agent add bobmatnyc/claude-mpm-skills pr-quality-checklisttype(scope): description{type}({scope}): {short description}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 workflowauthcartsearchapiuser-profileuser-profile-settings-modal## Summary
<!-- 1-3 bullet points describing what changed -->
- Added OAuth2 authentication flow
- Integrated with Auth0 provider
- Updated login UI components
## Related Tickets
<!-- Link to Linear/Jira/GitHub issues -->
- Closes #123
- Related to #456
- Fixes ENG-789
## Changes
<!-- Detailed list of 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
<!-- Required for UI changes -->
### Desktop

### Mobile

### Before/After (if applicable)
| Before | After |
|--------|-------|
|  |  |
## Testing
<!-- How was this tested? -->
- [ ] 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
<!-- If any 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' });
### Minimal Template (for small changes)
```markdown
## Summary
Brief description of the change.
## Changes
- Modified `file.ts` to fix XYZ
## Testing
- [ ] Tests pass
- [ ] Verified manually| Size | Lines | Files | Review Time | Recommendation |
|---|---|---|---|---|
| XS | < 50 | 1-3 | 5 min | ✅ Ideal for hotfixes |
| S | 50-150 | 3-8 | 15 min | ✅ Good size |
| M | 150-300 | 8-15 | 30 min | ⚠️ Consider splitting |
| L | 300-500 | 15-25 | 1 hour | ❌ Should split |
| XL | 500+ | 25+ | 2+ hours | ❌ Must split |
PR #1: MVP implementation (core functionality)
PR #2: Polish and edge cases
PR #3: Additional featuresPR #1: Database schema changes
PR #2: Backend API implementation
PR #3: Frontend UI integration
PR #4: Tests and documentationPR #1: Refactoring (no behavior change)
PR #2: New feature (builds on refactored code)
PR #3: Tests for new featurePR #1: High-risk changes (need careful review)
PR #2: Low-risk changes (routine updates)git mv## 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 introducedrggrep✅ Approve: Code is good, minor nits only
💬 Comment: Suggestions but not blockers
🔄 Request Changes: Issues must be fixed before merge❌ "Fix bug"
❌ "Updates"
❌ "WIP"
❌ "Changes from code review"
✅ "fix(auth): prevent duplicate login requests"
✅ "feat(cart): add coupon code support"❌ "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"❌ 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)❌ 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)❌ "Updated the header design"
(No screenshots)
✅ "Updated the header design"
[Desktop screenshot]
[Mobile screenshot]
[Before/After comparison]❌ "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"❌ console.log('user data:', user);
❌ debugger;
❌ // TODO: fix this later
❌ import { test } from './test-utils'; (unused)
✅ Clean code without debugging artifacts❌ New feature shipped without changelog entry
✅ .changeset/new-feature.md:
---
"@myapp/web": minor
---
Add OAuth2 login support with Auth0 integration{type}({scope}): {clear, concise description}