Code Review Checklist: Từ Junior đến Senior Developer

Năm đầu tiên làm dev, tôi review code như thế này:

“Dòng 45 thiếu dấu chấm phẩy. Dòng 67 nên đổi tên biến data thành userData. Reject.”

Sau 5 năm và 500+ PRs, tôi review như thế này:

“Logic này có vẻ đúng, nhưng tôi lo về edge case khi user chưa login. Có thể thêm test case cho trường hợp đó không? Ngoài ra, có thể extract function này ra để dễ test hơn.”

Đây là những gì tôi học được.

Mindset: Code Review không phải là War

Junior Reviewer Mindset

• Tìm lỗi để chứng tỏ mình giỏi
• Reject PR vì lý do nhỏ nhặt
• Comment mang tính phán xét

Kết quả: Team không thích bạn review code.

Senior Reviewer Mindset

• Giúp author improve
• Share knowledge
• Build better software together

Kết quả: Team học được nhiều, code quality tăng.

Checklist: 3 Levels of Review

Level 1: Functionality (Bắt buộc)

1. Logic có đúng không?

// Bad: Logic sai
public function calculateDiscount($total, $userType)
{
    if ($userType == 'vip') {
        return $total * 0.2; // 20% discount
    }
    return 0;
}

// Issue: VIP user với đơn > 1M không được discount thêm?
// Comment: "Theo requirement, VIP + đơn > 1M được 30%. Logic này chỉ cho 20%. Có đúng không?"

2. Edge cases được handle chưa?

// Bad: Không handle edge cases
public function getUserOrders($userId)
{
    return Order::where('user_id', $userId)->get();
}

// Issues:
// - User không tồn tại?
// - User có 10,000 orders → Memory issue?
// Comment: "Nên check user tồn tại trước. Và có thể cần pagination nếu orders nhiều."

3. Security issues?

// Bad: SQL Injection
public function search(Request $request)
{
    $keyword = $request->input('keyword');
    return DB::select("SELECT * FROM products WHERE name LIKE '%{$keyword}%'");
}

// Comment: "SQL Injection vulnerability. Nên dùng Query Builder hoặc prepared statements."

Level 2: Code Quality (Quan trọng)

1. Code có dễ hiểu không?

// Bad: Khó hiểu
public function process($d)
{
    $r = [];
    foreach ($d as $i) {
        if ($i['s'] == 1) {
            $r[] = $i;
        }
    }
    return $r;
}

// Comment: "Có thể rename variables để dễ hiểu hơn không? Ví dụ: $data, $result, $item, $status."

2. Có duplicate code không?

// Bad: Duplicate
public function getActiveUsers()
{
    return User::where('status', 'active')->where('deleted_at', null)->get();
}

public function getActivePosts()
{
    return Post::where('status', 'active')->where('deleted_at', null)->get();
}

// Comment: "Có thể tạo scope `active()` để reuse logic này?"

// Good:
trait ActiveScope
{
    public function scopeActive($query)
    {
        return $query->where('status', 'active')->whereNull('deleted_at');
    }
}

User::active()->get();
Post::active()->get();

3. Function quá dài?

// Bad: God function (100 dòng)
public function checkout(Request $request)
{
    // Validate
    // Calculate total
    // Apply discount
    // Process payment
    // Send email
    // Update inventory
    // Log activity
    // ...
}

// Comment: "Function này làm quá nhiều việc. Có thể extract thành các services riêng?"

// Good:
public function checkout(Request $request)
{
    $order = $this->orderService->create($request);
    $this->paymentService->process($order);
    $this->notificationService->sendConfirmation($order);
    return $order;
}

Level 3: Architecture (Nice to have)

1. Có follow design patterns không?

// Bad: Controller quá fat
class OrderController
{
    public function store(Request $request)
    {
        // 50 dòng business logic ở đây
    }
}

// Comment: "Có thể move business logic vào Service layer không? Controller chỉ nên handle HTTP."

// Good:
class OrderController
{
    public function store(Request $request, OrderService $service)
    {
        $order = $service->createOrder($request->validated());
        return response()->json($order, 201);
    }
}

2. Có testable không?

// Bad: Khó test
public function processOrder()
{
    $order = Order::find(request()->input('order_id'));
    Mail::to($order->user)->send(new OrderConfirmation($order));
}

// Comment: "Function này khó test vì depend vào global request() và Mail facade. Có thể inject dependencies?"

// Good:
public function processOrder(int $orderId, MailerInterface $mailer)
{
    $order = Order::findOrFail($orderId);
    $mailer->send($order->user->email, new OrderConfirmation($order));
}

Cách Give Feedback

Bad Feedback

❌ "Code này tệ quá."
❌ "Sao không dùng X? Ai cũng biết X tốt hơn Y."
❌ "Reject. Fix hết rồi submit lại."

Vấn đề: Mang tính phán xét, không constructive.

Good Feedback

✅ "Logic này có vẻ đúng, nhưng tôi lo về edge case X. Có thể thêm test case?"
✅ "Có thể dùng pattern X thay vì Y không? Lý do: [giải thích]. Tham khảo: [link]."
✅ "Approve với minor comments. Có thể fix trong PR tiếp theo."

Tại sao tốt:

• Explain “why”, không chỉ “what”
• Suggest, không command
• Acknowledge good parts

Những gì KHÔNG NÊN comment

1. Code Style (nếu có linter)

// ❌ Đừng comment:
"Dòng 45 thiếu space sau dấu phẩy."

// ✅ Thay vào đó:
Setup PHP-CS-Fixer:
composer require --dev friendsofphp/php-cs-fixer

Lý do: Linter làm tốt hơn con người, và không gây conflicts.

2. Personal Preference

// ❌ Đừng comment:
"Tôi thích dùng foreach hơn map."

// ✅ Chỉ comment nếu:
"map() có thể gây performance issue với array lớn. Có thể dùng foreach?"

Lý do: Preference không phải standard. Chỉ comment nếu có impact thực sự.

3. Nitpicking khi PR đã good enough

// ❌ Đừng block PR vì:
"Biến này nên đổi tên từ `data` thành `userData`."

// ✅ Thay vào đó:
"Approve. Minor: Có thể đổi tên biến cho rõ nghĩa hơn trong PR sau."

Lý do: Perfect là kẻ thù của good. Đừng block team vì perfectionism.

Khi nào Approve, khi nào Request Changes?

Approve khi:

• Logic đúng
• Tests pass
• Không có security issues
• Minor issues có thể fix sau

Request Changes khi:

• Logic sai hoặc thiếu edge cases
• Security vulnerabilities
• Breaking changes không được document
• Tests fail hoặc thiếu tests quan trọng

Comment (không block) khi:

• Code style issues (nếu không có linter)
• Suggestions để improve (không bắt buộc)
• Questions để hiểu rõ hơn

Template Review Comment

## Summary
[Tóm tắt PR làm gì]

## Good Parts
- [Điểm tốt 1]
- [Điểm tốt 2]

## Concerns
1. **[Issue 1]**
   - Location: `file.php:45`
   - Issue: [Mô tả vấn đề]
   - Suggestion: [Đề xuất fix]
   - Why: [Giải thích tại sao]

2. **[Issue 2]**
   ...

## Questions
- [Câu hỏi để clarify logic]

## Minor/Optional
- [Suggestions không bắt buộc]

## Decision
- [ ] Approve
- [ ] Approve with comments
- [ ] Request changes

Checklist cho Reviewer

Trước khi submit review:

• Đã đọc description và hiểu mục đích của PR?
• Đã pull code về và test locally?
• Đã check tests pass?
• Đã check security issues?
• Feedback có constructive không?
• Đã acknowledge good parts?
• Đã explain “why” cho mọi comment?
• Có block PR vì lý do nhỏ nhặt không?

Tools giúp việc

1. GitHub/GitLab Suggestions

```suggestion
// Code suggestion ở đây

Author có thể apply suggestion bằng 1 click.

**2. Automated Checks**

```yaml
# .github/workflows/pr-checks.yml
name: PR Checks
on: [pull_request]
jobs:
  lint:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - run: composer install
      - run: ./vendor/bin/php-cs-fixer fix --dry-run
  test:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - run: composer install
      - run: php artisan test

3. Danger.js

Tự động comment những issues phổ biến:

// dangerfile.js
import { danger, warn } from 'danger';

// Warn nếu PR quá lớn
if (danger.github.pr.additions > 500) {
  warn('PR này lớn quá. Có thể split thành nhiều PRs nhỏ hơn?');
}

// Warn nếu thiếu tests
const hasTests = danger.git.created_files.some(f => f.includes('test'));
if (!hasTests) {
  warn('PR này không có tests. Có cần thêm không?');
}

Kết luận

Code review không phải về việc tìm lỗi. Nó về việc:

• Giúp team improve
• Share knowledge
• Build better software
• Build better team culture

Key takeaways:

• Focus vào functionality và security trước
• Give constructive feedback
• Explain “why”, không chỉ “what”
• Approve khi good enough, không đợi perfect
• Automate những gì có thể (linting, testing)

Lời khuyên cuối:

• Review code của người khác như bạn muốn code của mình được review
• Luôn assume good intent
• Học từ mọi PR (kể cả khi bạn là reviewer)

Good code review builds good teams.