Review kém hiếm khi vì thiếu trình độ, mà thường vì thiếu tiêu chí rõ ràng và PR quá lớn. Một pattern hay gặp: reviewer nhận PR 800 dòng, đọc 30 phút, comment vài chỗ naming, approve. Tuần sau production bug từ chính PR đó, race condition ở handler không được catch vì working memory đã hết sau khi đọc 600 dòng refactor không liên quan. Working memory có giới hạn, nếu dùng hết cho việc hiểu code thì không còn cho việc phát hiện bug.
Bài này đi vào cách approach code review có hệ thống: rubric để biết kiểm gì trước, sizing để giữ PR trong ngân sách nhận thức, và template comment để giao tiếp rõ ràng giữa reviewer và author.
Vì sao cần rubric
Không có rubric, review rơi vào hai thái cực. Một là “LGTM” vô điều kiện vì deadline, reviewer scroll nhanh, không catch được gì. Hai là nitpick vô tận vì style cá nhân, 20 comment về naming convention, 0 comment về logic bug.
Rubric, dù chỉ trong đầu, giúp reviewer biết cần focus ở đâu trước. Review theo thứ tự ưu tiên, dừng ở tầng nào mà tầng trước chưa rõ.
Bốn tầng kiểm
Correctness và safety
Đây là tầng quan trọng nhất, logic đúng không, null/undefined handle chưa, race condition, migration backward compatible. Nếu correctness chưa rõ, không nên dành pixel cho style.
Nên check: edge case input (empty, null, max length), concurrent access (hai request cùng lúc), error handling (fail thì sao, retry thì sao), data migration (code mới đọc data cũ được không, code cũ đọc data mới được không trong rolling update).
Security
Authorization check đúng path, input validation, không log secret/PII, SSRF surface. Với PR đụng auth hoặc payment, ba câu hỏi tối thiểu: có thay đổi trust boundary không, có log sensitive data không, có test deny cho authz không. Ba câu này bắt được nhiều lỗ hổng “ngớ ngẩn nhưng đắt”, user A đọc được data user B vì thiếu authz check.
Performance và reliability
N+1 query, unbounded loop, retry storm, missing timeout. Không cần optimize mọi thứ, nhưng performance bug có thể bring down production. Check: query trong loop, pagination thiếu limit, retry không có backoff, connection/pool leak.
Maintainability
Naming, module boundary, test đọc được, code structure. Đây là tầng cuối cùng, quan trọng nhưng không quan trọng bằng ba tầng trước. Nếu correctness và security chưa clear, đừng dành thời gian cho naming.
Thực tế nhiều review session 80% comment là maintainability (naming, format, style) trong khi có bug logic hoặc security issue chưa được catch. Rubric giúp flip tỷ lệ đó.
PR sizing: ngân sách nhận thức
PR quá lớn (hơn 400 dòng diff) → xác suất reviewer bỏ sót tăng nhanh. Đây không phải ý kiến cá nhân, Google đã nghiên cứu nội bộ và publish kết quả tương tự.
Nhưng “PR nhỏ” không phải dogma. Quan trọng là một PR một mục tiêu review, reviewer biết rõ đang review gì. PR refactor thuần (rename, move) + PR behavior change nên tách. PR format toàn repo + feature mới nên tách. Khi không tách được, description phải compensate bằng giải thích chi tiết hơn.
Heuristic hiệu quả: dưới 200 dòng net (trừ test, generated) là ideal. 200-400 chấp nhận. Trên 400 nên có lý do rõ ràng (migration, bulk rename).
Template comment
Comment không có prefix → author không biết phải fix hay không. Convention phổ biến:
[blocking] Race condition ở getOrders khi hai request concurrent
→ cần lock hoặc check version. Đề xuất: SELECT FOR UPDATE.
[question] Vì sao dùng setTimeout thay vì scheduler ở đây?
Có concern gì với approach khác không?
[nit] Tên biến `d` → `duration` đọc rõ hơn. Không block merge.
Blocking = phải fix trước merge. Question = cần author giải thích (có thể không cần fix). Nit = preference, không block.
Phân loại giúp author biết phải focus gì, không phải đoán “comment này có block merge không?”.
Design note
Bắt buộc khi thay đổi: contract API (request/response schema), database schema (migration), authorization model, hoặc infra cost đáng kể. Design note ngắn trong PR description hoặc comment đầu tiên, không cần document riêng, nhưng cần ghi lại trade-off và lý do.
Async vs pair review
Phần lớn review là async, comment trên GitHub/GitLab, phản hồi khi có thời gian. Ưu điểm: ghi lại được (artifact cho team), scale timezone. Nhược điểm: dễ kéo dài ping-pong (comment → fix → comment → fix, mất 3 ngày cho thứ có thể resolve trong 15 phút).
Với PR rủi ro cao, cách hiệu quả nhất thường là 15 phút pair trước khi merge thay vì 30 comment qua lại. Author walkthrough code, reviewer hỏi real-time, resolve ngay. Ghi summary vào PR sau khi pair, giữ artifact cho team.
Review theo mức rủi ro PR
Không phải mọi PR cần cùng mức review. Phân loại theo rủi ro:
Low risk (docs, typo, copy): CI xanh + một reviewer approve. Không cần overthink.
Medium risk (feature nội bộ, refactor): rubric đủ 4 tầng, test tự động liên quan pass. Một reviewer đủ.
High risk (auth, billing, data migration): hai reviewer hoặc một senior theo policy team. Rollback plan hoặc feature flag được nhắc trong PR description. Authz matrix check (ít nhất mentally) cho path mới.
Policy phải viết rõ, không phải quy tắc ngầm mà senior “biết” còn junior không biết.
SLA phản hồi
PR chờ review quá lâu tạo frustration cho author, context switch cost khi reviewer cuối cùng comment, và branch diverge dẫn đến conflict.
SLA recommend (không cần tool, cần thỏa thuận team):
Hotfix: dưới 30 phút (tag reviewer, mention channel). PR nhỏ: dưới 1 ngày làm việc. PR lớn: acknowledge trong 4 giờ + schedule pair nếu cần.
Cần người backup khi reviewer chính bận, không để PR stuck vì một người nghỉ phép.
Comment culture và tâm lý an toàn
Viết comment về code, không về con người. “Lo case null ở đây có thể crash” thay vì “sao bạn không nghĩ đến null?”. Nghe giống nhau nhưng tone khác hoàn toàn.
Dùng “chúng ta” hoặc ngôi chung: “chưa hiểu flow ở đây” thay vì “code này khó hiểu”. Hỏi thay vì phán: “có lý do dùng approach này không?” thay vì “approach này sai”.
Khi author push fix, reviewer resolve thread hoặc giải thích vì sao chưa đủ, tránh thread treo vô hạn mà không ai biết status. Thread treo = ambiguity = author không biết có cần fix thêm không.
Chỉ ra điều tốt cũng quan trọng, “cách xử lý error ở đây clean!” khuyến khích pattern tốt lan rộng. Review chỉ toàn negative → author dreading mỗi lần mở PR.
Reviewer không phải gatekeeper
Reviewer tốt tăng throughput team: phản hồi nhanh, phân loại rõ (blocking vs nit), đề xuất diff cụ thể. Reviewer xấu trở thành gatekeeper: mọi PR phải theo gu cá nhân không viết thành văn, approve chậm, comment mơ hồ.
Rubric nên là tài sản team (ADR ngắn, checklist trong wiki), không phải truyền miệng. Khi rubric viết rõ, junior có thể review senior đúng chỗ (“em không thấy test deny cho role X theo matrix”), đó là văn hóa kỹ thuật, không phải hierarchy.
Security review nhẹ trong PR
Không cần chờ security audit riêng cho mỗi PR. Với PR đụng auth/payment, ba câu hỏi đủ:
Có thay đổi trust boundary không? Có log secret hoặc PII không? Có test deny cho authorization không?
Integrate ba câu này vào review checklist cho high-risk PR. Không tốn thêm thời gian đáng kể nhưng catch được nhiều lỗi bảo mật “tầm thường” trước khi lên production.
Approve with comment: khi nào OK
Approve mà vẫn có comment chưa resolve, chấp nhận được khi comment còn lại là nit hoặc follow-up đã tạo ticket (link Jira/GitHub issue). Không chấp nhận khi còn blocking chưa có cam kết fix, trừ khi policy cho phép merge + hotfix branch (ghi rõ trong PR).
AI-assisted review
LLM có thể tóm tắt diff, spot pattern (N+1, missing null check), suggest improvement. Nhưng AI không thay thế trách nhiệm authz/security review, AI không hiểu business context, trust boundary cụ thể, hay authorization model của hệ thống.
Policy team nên coi AI review như thêm một reviewer tham khảo, output vẫn cần người chịu trách nhiệm merge.
Đo chất lượng review
Mỗi quý, hội ý nhanh với team: PR lớn nhất merge được là bao nhiêu dòng, median thời gian first response, có PR nào revert trong 48 giờ không (vì sao). Không cần dashboard phức tạp, spreadsheet 10 dòng vẫn đủ để thấy pattern.
PR revert trong 48 giờ là signal quan trọng nhất, nghĩa là review không catch issue đủ nghiêm trọng để cần revert. Hiếm thì bình thường, thường xuyên thì cần cải thiện review process.
Tóm tắt
Rubric (correctness → security → performance → maintainability) giữ review focus đúng thứ tự ưu tiên. Đừng dành 80% comment cho naming khi còn logic bug.
PR sizing là lever lớn nhất, dưới 400 dòng, reviewer còn working memory để catch bug. Một PR một mục tiêu review.
Template comment (blocking/question/nit) giúp author biết phải focus gì. Design note bắt buộc khi đổi API/schema/authz. SLA phản hồi (1 ngày cho PR nhỏ) giữ throughput.
Comment culture: về code không về người, hỏi thay vì phán, resolve thread rõ ràng, chỉ ra điều tốt. Rubric viết thành văn để junior review senior đúng chỗ.
Security review nhẹ trong PR: 3 câu hỏi (trust boundary, log PII, test deny) catch nhiều lỗi bảo mật trước production.