code-review-checklist Templates

# Code Review: [PR title or filename]

## Summary

**Overall**: ⭐⭐⭐⭐☆ (4/5)

**Reviewer**: [name]
**Date**: [YYYY-MM-DD]

| Category | Score | Key Issues |
|----------|-------|------------|
| 🏗️ Architecture | ✅ | - |
| 🧩 State Management | ⚠️ | BlocSignalBuilder optimization needed |
| 🔒 Security | ✅ | - |
| ⚡ Performance | ⚠️ | Image cache size not specified |
| 🧪 Testing | ❌ | UseCase tests missing |
| 📖 Readability | ✅ | - |
| 🌐 i18n | ✅ | - |
| ♿ Accessibility | ⚠️ | Some semantic labels missing |
| 🧱 Structural Failure Modes | ⚠️ | New endpoint method has no authorization guard |

---

## 🔴 Critical Issues (must fix before merge)

### 1. [Security] API key hardcoded
- **File**: `lib/core/config.dart:15`
- **Problem**: API key directly included in source code
- **Impact**: Security vulnerability, key exposure risk
- **Fix**: Use environment variables via Envied

### 2. [Testing] UseCase tests missing
- **File**: `test/domain/usecase/`
- **Problem**: No tests for core business logic
- **Impact**: Risk of regression bugs
- **Fix**: Add unit tests per UseCase

---

## 🟡 Improvements (recommended fixes)

### 1. [Performance] Image cache size needs to be specified
- **File**: `lib/presentation/widget/product_card.dart:42`
- **Current code**:

- **Recommended code**:

### 2. [State Management] BlocSignalBuilder optimization
- **File**: `lib/presentation/page/home_page.dart:78`
- **Current code**:

- **Recommended code**:

---

## 🟢 Suggestions (optional improvements)

### 1. [Readability] Variable name improvement suggestion
- `data` → `userProfile`
- `list` → `productItems`
- `result` → `authResult`

### 2. [Structure] Widget extraction suggestion
- `_buildHeader()` → Extract to `HomeHeader` widget
- Improves reusability and testability

---

## ❓ Questions

1. `auth_bloc.dart:45` - What is the intent of this exception handling logic?
2. `user_repository.dart:89` - Is there a reason the cache expiration time is 5 minutes?

---

## Conclusion

Overall good code. Can merge after fixing 2 Critical Issues.

# Quick Review: [filename]

**Result**: ✅ Approved with comments

## Key Feedback

1. **[Performance]** Adding image cache size recommended
   - `product_card.dart:42`

2. **[Readability]** Variable name `data` → `userProfile` recommended
   - `home_bloc.dart:25`

## Well Done 👍

- Clean Architecture pattern well followed
- Immutable state management using Freezed
- Proper error handling

# Detailed Review: [category name]

## 🏗️ Architecture Review

### Review Items

| Item | Status | Notes |
|------|--------|-------|
| Clean Architecture layer separation | ✅ | |
| UseCase pattern usage | ✅ | |
| Repository interface separation | ⚠️ | Partially missing |
| Feature module independence | ✅ | |

### Detailed Feedback

#### Repository Interface Separation (⚠️)

**Current State**:
- `CartRepository` directly implemented without interface

**Recommended Change**:
1. Create `ICartRepository` interface
2. Place interface in Domain layer
3. Implement in Data layer

🔴 **[Required]** Security vulnerability

**Location**: `lib/core/config.dart:15`

**Problem**: API key is hardcoded.

**Fix**:

Cannot merge until this issue is resolved.

🟡 **[Improvement]** Performance optimization

**Location**: `lib/presentation/widget/product_card.dart:42`

**Current code**:

**Recommended code**:

Specifying image cache size reduces memory usage.

🟢 **[Suggestion]** Variable name improvement

**Location**: `lib/presentation/bloc/home_bloc.dart:25`

Changing variable name `data` to `userProfile` would improve readability.

---

❓ **[Question]** Exception handling intent

**Location**: `lib/data/repository/auth_repository.dart:45`

Is there a reason this try-catch block ignores all exceptions?

✨ **Nice!** Clean state management

**Location**: `lib/presentation/bloc/cart_bloc.dart`

State management using Freezed and Union Types is very clean.
The loading/error/success state separation is especially well done!

# Self-Review Checklist

Self-check list before PR submission.

## Basics

- [ ] Code builds successfully
- [ ] All tests pass
- [ ] No lint errors
- [ ] Related issues are linked

## Architecture

- [ ] Clean Architecture layer separation followed
- [ ] No direct dependencies between feature modules
- [ ] New code follows existing patterns

## Security

- [ ] No hardcoded secrets
- [ ] No sensitive information in logs
- [ ] Input validation applied

## Performance

- [ ] const widgets used
- [ ] Image cache size specified
- [ ] Resources released in dispose

## Testing

- [ ] Tests added for new features
- [ ] Edge cases covered

## Internationalization

- [ ] No hardcoded strings
- [ ] Translation keys used

## Final Check

- [ ] Unnecessary console.log/print removed
- [ ] TODO comments resolved or issues filed
- [ ] Commit messages follow convention

LGTM! 🎉

All checklist items verified.

**Verified items**:
- ✅ Build successful
- ✅ Tests passing
- ✅ No security issues
- ✅ Code quality good

Great work!

Changes requested.

**Required fixes** (must resolve before merge):
1. Remove hardcoded API key (`config.dart:15`)
2. Add UseCase tests

**Recommended fixes** (optional):
1. Specify image cache size
2. BlocSignalBuilder optimization

Please re-request review after fixing the required items.

Overall looks good. A few questions/suggestions.

**Questions**:
1. Curious about the caching strategy choice

**Suggestions**:
1. Please consider error message internationalization

Please respond after review.