Anti-pattern trong code review và metric nên đo Trung cấp

Trang này trả lời: review hỏng trông ra sao, đo gì để thấy nó mà không làm nó tệ hơn, và khi nào được phá quy tắc.

Anti-pattern

Đang tải diagram…

Anti-pattern	Dấu hiệu	Cách sửa
Rubber stamp	Approve PR 800 dòng sau 3 phút, không comment nào	Reviewer chịu trách nhiệm như tác giả (Google: reviewer có “ownership and responsibility”); không đọc kỹ được thì nói “mình chỉ xem phần X”
Gatekeeping	Một người phải approve mọi PR; chặn vì “anh không làm thế”	CODEOWNERS nhiều người; chặn phải dựa trên nguyên tắc hoặc dữ liệu
Nitpick storm	20 nit: đè lên một issue thật	Style vào linter; giới hạn nit; comment blocking lên đầu
Review PR 2.000 dòng	“LGTM” vì bỏ cuộc	Yêu cầu chia (stacking, vertical slice); ít nhất review thiết kế rồi trả lại
Viết lại PR trong comment	Reviewer dán 80 dòng code “của mình”	Cần thiết kế khác: call, rồi tác giả viết. Code nhỏ thì dùng “suggested change”
Bike-shedding	Thread dài nhất PR là về tên một biến cục bộ	Nhãn nitpick, để tác giả quyết
Approve có điều kiện mập mờ	“LGTM nhưng xem lại phần retry”	Hoặc blocking, hoặc nói rõ không cần xem lại
“Dọn ở PR sau”	TODO không có ticket, flag thứ ba trong hàm	Độ phức tạp mới dọn ngay; vấn đề cũ có ticket và người nhận

Metric: đo gì cho lành mạnh

Metric	Lành mạnh khi dùng để	Độc hại khi
Thời gian tới phản hồi đầu tiên	Phát hiện PR bị bỏ quên; mục tiêu dưới 1 ngày làm việc	—
Thời gian từ mở PR tới merge (p50, p90)	Thấy nút thắt của cả quy trình	So giữa các cá nhân
Kích thước PR (p50, p90)	Theo dõi xu hướng PR phình to	Hạn mức cứng làm người ta chia PR vô nghĩa
Số vòng review	PR quay lại nhiều lần → mô tả hoặc thiết kế chưa rõ từ đầu	Xếp hạng “người bị comment nhiều nhất”
Số comment mỗi reviewer	—	KPI: sinh nitpick storm
Số PR mỗi người	—	KPI: sinh PR nhỏ vô nghĩa, chia nhỏ giả
Tỷ lệ “approve không comment”	Tín hiệu rubber stamp nếu tăng đột biến trên PR lớn	Ép comment cho có

Nguyên tắc: đo quy trình (độ trễ, kích thước, số vòng), không đo người. Metric gắn với hiệu suất cá nhân sẽ bị tối ưu theo cách phá hỏng review.

Yêu cầu 2 approve: được gì

• Phù hợp cho code tiền, quyền truy cập, migration production.

• Hai góc nhìn, giảm rủi ro một người lướt.

• Gấp đôi thời gian chờ; dễ sinh “người thứ hai approve vì người thứ nhất đã approve”.

• Với phần lớn service, một reviewer phù hợp + test tốt hiệu quả hơn hai reviewer hời hợt.

Yêu cầu 2 approve: mất gì

Khi nào áp dụng / ngoại lệ

Quy tắc	Ngoại lệ hợp lý
PR nhỏ, một mục đích	Xoá cả file, code sinh tự động, refactor bằng công cụ tin cậy (Google coi là chấp nhận được), lockfile
Dọn độ phức tạp trước khi merge	Hotfix trong sự cố: merge bản sửa tối thiểu, mở ticket dọn ngay sau đó
Phản hồi trong 1 ngày làm việc	Đang trực sự cố hoặc nghỉ: hệ thống phải tự gán reviewer khác
Review trước khi merge	Pair/mob programming toàn thời gian đã là review liên tục; vẫn nên có PR để CI chạy và để lại lịch sử
Comment theo Conventional Comments	Team nhỏ đã có quy ước nit:/blocking: riêng: giữ quy ước đang chạy tốt

Tiếp theo: Lỗi hay gặp.