ADR, review code và xử lý bất đồng trong review

Đó chính là việc của ADR (Architecture Decision Record): một file ngắn ghi lại một quyết định kiến trúc, bối cảnh lúc đó và hệ quả.

1. Bài toán

6 tháng sau, dev mới hỏi:
  "Sao refund đơn cũ lại đi qua Provider A? Xóa đi cho gọn được không?"
Không ai nhớ. Người quyết định đã nghỉ.
Xóa → refund đơn cũ hỏng → incident.

2. Vấn đề

• Quyết định nằm trong Slack, meeting, đầu người. Không tìm lại được.
• Không biết các phương án đã bị loại và vì sao, nên team tranh luận lại từ đầu.
• Không biết điều kiện nào thay đổi thì nên xem lại quyết định.

3. Giải pháp: ADR

• Lưu trong repo: docs/adr/0007-payment-provider-b.md. Review như code.
• Không sửa ADR cũ khi đổi ý. Viết ADR mới, đánh dấu cái cũ Superseded by 0012.
• Ngắn: 1 trang. Chỉ cho quyết định khó đảo ngược hoặc ảnh hưởng nhiều team.

4. Template và ví dụ

Template ADR

# ADR-NNNN: <Tiêu đề quyết định>
Status: Proposed | Accepted | Deprecated | Superseded by ADR-XXXX
Date: YYYY-MM-DD
Deciders: <tên>

## Context
Vấn đề, ràng buộc, số liệu. Vì sao phải quyết bây giờ.

## Options considered
1. ... (ưu / nhược)
2. ...

## Decision
Chúng ta chọn ... vì ...

## Consequences
Tốt: ...
Xấu / chấp nhận: ...
Xem lại khi: ...

Ví dụ thật trên shop-app

# ADR-0007: Chuyển checkout sang Provider B, giữ A cho refund cũ
Status: Accepted
Date: 2026-09-02
Deciders: Lan (tech lead), Minh (PM), team payment

## Context
- Hợp đồng Provider A hết hạn 30/11, phí B thấp hơn 0.4%/giao dịch.
- 12.000 đơn đã thanh toán qua A còn trong hạn refund 60 ngày.
- B không hỗ trợ refund giao dịch tạo ở A.

## Options considered
1. Big-bang chuyển hết sang B: nhanh, nhưng không rollback được, refund cũ hỏng.
2. Feature flag chuyển dần checkout sang B, refund theo provider gốc: an toàn, thêm code.
3. Gia hạn A 6 tháng: tốn ~X USD phí, chỉ trì hoãn vấn đề.

## Decision
Chọn 2. PaymentGateway chọn provider theo order.provider.
Rollout 5% → 50% → 100% qua flag checkout.provider-b.

## Consequences
Tốt: rollback trong 1 phút, refund cũ an toàn.
Xấu: giữ 2 adapter đến hết 01/2027; phải giữ credential A (qua ${PROVIDER_A_API_KEY}).
Xem lại khi: không còn đơn A trong hạn refund → xóa adapter A (ADR mới).

Code review: nhìn gì, theo thứ tự

Giọng comment: gắn nhãn mức độ

[blocking] Webhook handler chưa check eventId đã xử lý chưa.
           Provider B retry tối đa 5 lần → có thể ghi nhận payment 2 lần.
           Gợi ý: bảng processed_events với unique(event_id).

[question] Sao mình dùng REQUIRES_NEW ở đây? Mình chưa rõ ý định.

[suggestion] Có thể tách mapStatus() ra cho dễ test, không bắt buộc.

[nit] Tên biến resp → providerResponse.

[praise] Test cho case timeout rất rõ, cảm ơn nhé.

Nguyên tắc tone:

• Nói về code, không nói về người: "đoạn này có thể…" thay vì "bạn lại quên…".
• Giải thích vì sao và gợi ý cách sửa.
• Hỏi khi không chắc, đừng khẳng định.
• Khen chỗ làm tốt, cụ thể.
• PR quá lớn (> 400 dòng): đề nghị tách, đừng cố review hời hợt.
• Review trong vòng 1 ngày làm việc. Chậm review là chặn cả team.

5. Cách hoạt động: bất đồng trong review

Bất đồng trên PR
      |
      v
Là style/ý thích? ──yes──> Theo convention/linter. Không có? Người viết quyết.
      | no
      v
Comment qua lại > 2 vòng? ──yes──> Gọi 15 phút nói trực tiếp
      | no                              |
      v                                 v
Thảo luận bằng dữ liệu         Vẫn chưa thống nhất?
(benchmark, doc, incident)              |
                                        v
                       Hỏi người thứ 3 / tech lead, hoặc viết ADR
                                        |
                                        v
                       Quyết xong → "disagree and commit", ghi lại kết quả lên PR

6. So sánh

7. Khi nào nên / không nên

• Nên viết ADR: chọn DB, message broker, provider, chiến lược migration, đổi kiểu auth.
• Không cần ADR: chọn tên biến, thêm một endpoint, nâng patch version.
• Review không nên: chặn PR vì nit, viết lại toàn bộ theo ý mình.

Không nên comment từng chỗ. 80 comment trên một PR khổng lồ làm junior nản và bạn vẫn sót bug. Làm thế này:

1. Comment một lần ở đầu: "PR lớn quá nên mình khó review kỹ. Mình tách thành 3 PR được không: (1) refactor không đổi hành vi, (2) entity + migration, (3) API coupon?"
2. Gọi 15 phút, cùng junior chia PR. Đây là dịp mentor, không phải phạt.
3. Thêm vào PR template: "PR có > 400 dòng không? Có trộn refactor với feature không?"

Giải quyết nguyên nhân (quy trình), không chỉ triệu chứng (PR này).

Đúng rồi, nhiều người ngại, nhưng review không phân cấp bậc. Bug của tech lead vẫn lên production như bug của ai khác. Mẹo:

• Dùng [question] và kèm bằng chứng: "Mình hiểu là @Transactional trên private method sẽ không có hiệu lực vì proxy, mình hiểu đúng không anh?"
• Nếu là rủi ro lớn (mất tiền, mất dữ liệu), nói thẳng và đánh [blocking]. Lịch sự nhưng không mơ hồ.
• Tech lead tốt sẽ quý bạn hơn. Nếu không, đó là thông tin về văn hóa team.

Chi tiết proxy và transaction, xem lại chương Spring Core / Spring JPA.

Q1. Bạn review code thế nào?

🇻🇳 Tiếng Việt:

Em đọc mô tả PR và ticket trước để hiểu mục tiêu. Em ưu tiên đúng đắn, security, vận hành như migration và rollback, rồi tới test, sau cùng mới tới thiết kế và tên biến. Format thì để linter lo. Em gắn nhãn blocking, question, nit để người viết biết cái nào bắt buộc, luôn giải thích lý do và khen chỗ làm tốt. PR quá lớn thì em đề nghị tách.

🇬🇧 English:

I read the PR description and ticket first to understand the goal. I focus on correctness, security and operational concerns like migrations and rollback, then tests, then design and naming. Formatting is left to the linter. I label comments as blocking, question or nit so the author knows what is required, I always explain why, and I call out good work. If a PR is too large I ask to split it.

Q2. Kể lần bạn bất đồng kỹ thuật với đồng nghiệp.

🇻🇳 Tiếng Việt:

Khi chuyển payment provider, một đồng nghiệp muốn chuyển big-bang trong một đêm cho đơn giản. Em lo không rollback được và refund đơn cũ sẽ hỏng. Sau hai vòng comment, em đề nghị gọi 15 phút, mỗi người liệt kê rủi ro, rồi em viết ADR so sánh ba phương án. Team chọn feature flag. Đồng nghiệp đó đóng góp ý tưởng rollout theo phần trăm. Kết quả chuyển xong không có incident.

🇬🇧 English:

During a payment provider migration, a colleague wanted a one-night big-bang switch for simplicity. I was worried we couldn't roll back and old refunds would break. After two rounds of PR comments I suggested a 15-minute call, we each listed risks, and I wrote an ADR comparing three options. The team chose a feature-flag rollout, and my colleague contributed the percentage rollout idea. We migrated with zero incidents.

Q3. ADR là gì, bạn dùng khi nào?

🇻🇳 Tiếng Việt:

ADR là tài liệu ngắn trong repo ghi một quyết định kiến trúc: bối cảnh, các phương án, quyết định và hệ quả. Em viết cho quyết định khó đảo ngược như chọn provider hay chiến lược migration. Đổi ý thì viết ADR mới và đánh dấu cái cũ là superseded.

🇬🇧 English:

An ADR is a short document in the repo that records one architectural decision: context, options, the decision and consequences. I write one for hard-to-reverse choices like a payment provider or a migration strategy. When we change our mind we add a new ADR and mark the old one as superseded.

Cách nhớ ngắn gọn

• ADR = Context → Options → Decision → Consequences, 1 trang, trong repo.
• Review: đúng → an toàn → vận hành → test → thiết kế → tên.
• Gắn nhãn blocking / question / nit / praise.
• Bất đồng > 2 vòng → nói trực tiếp → dữ liệu → disagree and commit.