Review code

Technical

Review để nâng chất lượng và nâng người, không chỉ bắt lỗi.

Bạn đang ở đâu với kỹ năng này?

Làm bài Đánh giá Kỹ năng Kỹ thuật cho PM/BA — 2 người đã làm. Một lần làm chấm điểm cho tất cả kỹ năng trong nhóm Technical, không chỉ kỹ năng này.

Làm bài đánh giá

Kỹ năng này cho bạn điều gì

Bắt đầu từ đây

Review code có hai mục tiêu và chúng hay mâu thuẫn: giữ chất lượng, và nâng người viết. Review chỉ bắt lỗi thì code sạch hơn nhưng người viết không khá lên. Nguyên tắc nền: nhận xét về CODE, hỏi về ý định, và phân biệt “phải sửa” với “nếu là tôi thì tôi làm khác”.

Khi đã làm được

Review được PR lớn trong thời gian hợp lý mà không thành nút thắt: phân loại nhận xét theo mức bắt buộc, trả lời trong vòng một ngày làm việc, biết khi nào nên gọi điện thay vì viết comment thứ mười lăm, và dùng review để truyền chuẩn thay vì để chứng minh mình đúng.

Roadmap — Cách học và đạt kỹ năng

Đã đọc 0/14 mục · còn khoảng 5 phút

Cách chúng tôi ghi nhận

Một mục được tính là đã đọc khi nó nằm giữa màn hình đủ lâu so với độ dài của nó (bảng và sơ đồ tính một nửa), bạn vẫn đang cuộn hoặc bấm phím, và bạn đã thấy tới cuối mục. Lướt nhanh hoặc để tab mở không được tính. Chúng tôi chỉ lưu thời gian đọc từng mục — không lưu gì khác.

Review code là gì?

Review code là đọc thay đổi của người khác trước khi nó vào nhánh chính, để bắt lỗi và để cả hai cùng học.

Với PM/BA, bạn hiếm khi review code — nhưng bạn chịu hậu quả của review tệ: PR nằm ba ngày là tính năng trễ ba ngày, và không ai báo cáo điều đó.

Ba mức nhận xét — ghi rõ mức, đừng để người đọc đoán

Mức	Nghĩa là	Viết thế nào
Blocking	Không sửa thì không merge	“Blocking: chỗ này null sẽ crash khi…”
Nên sửa	Nên làm, nhưng không chặn	“Nên: tách hàm này ra cho dễ test.”
Ý kiến	Sở thích cá nhân, bỏ qua được	“Nit: mình hay đặt tên là…”

Không ghi mức là nguyên nhân số một của cãi nhau trong review: người viết không biết cái nào bắt buộc.

Khi nào bạn gặp chuyện này

Tình huống	Bạn làm gì	Nếu không
Một PR nằm ba ngày không ai review.	Đặt luật: review trong một ngày làm việc, hoặc nói rõ khi nào sẽ review.	Người viết mất context, phải rebase, và lần sau họ gộp PR to hơn nữa.
Review có 27 comment, toàn về đặt tên biến.	Gộp thành một comment “nit” duy nhất, và tập trung vào logic và edge case.	Người viết thấy bị soi, và học được rằng review là chỗ bị phán xét.
Cùng một lỗi bị nhắc trong năm PR liên tiếp.	Chuyển thành luật lint hoặc một dòng trong checklist — đừng dựa vào trí nhớ người review.	Bạn tiêu thời gian review vào việc máy làm được, và vẫn sót lỗi thật.

Cách review một PR trong 20 phút

Đọc mô tả trước, code sau. Không hiểu PR này để làm gì thì chưa review được.
Chạy thử nếu có thể. Nhiều lỗi chỉ thấy khi chạy.
Đi từ ngoài vào: API/giao diện đúng chưa, rồi mới tới bên trong.
Hỏi thay vì phán. “Chỗ này khi X thì sao?” tốt hơn “Chỗ này sai”.
Ghi mức cho từng nhận xét — blocking, nên, hay nit.

Checklist review — dán vào PR template

Nhóm	Câu hỏi	Bỏ qua được không
Đúng việc	PR này có làm đúng thứ mô tả không?	Không
Edge case	Rỗng, null, quá dài, gọi hai lần thì sao?	Không
Lỗi và log	Khi hỏng thì ai biết, và biết bằng cách nào?	Không
Test	Có test cho phần logic mới không?	Không
Bảo mật	Dữ liệu người dùng có bị lộ ra log hay response không?	Không
Đọc được	Ba tháng sau người khác hiểu được không?	Có, nếu gấp
Đặt tên, style	Có theo chuẩn của dự án không?	Có — để lint lo

Sai lầm thường gặp

Sai lầm	Vì sao hỏng	Làm thay bằng
Không ghi mức nhận xét	Người viết không biết cái nào bắt buộc, và sửa hết hoặc cãi hết.	Blocking / Nên / Nit, ghi rõ.
Review kiểu “nếu là tôi”	Biến review thành tranh luận sở thích.	Chỉ chặn khi có hậu quả thật.
PR quá to	Không ai review nổi 2000 dòng; họ sẽ bấm approve.	Đặt trần ~400 dòng, tách PR.
Chỉ một người review được	Người đó thành nút thắt và không dám nghỉ.	Luân phiên, và viết chuẩn ra để người khác review được.

Vì sao vai trò của bạn cần cái này

Vai trò	Bạn dùng nó để
Engineering Manager	Thời gian PR chờ review là một trong vài chỉ số dự báo tốc độ đội tốt nhất mà bạn có.
Product Manager	Bạn không review code, nhưng PR tồn đọng là lý do thật đằng sau “sprint này chậm” — biết để hỏi đúng câu.
Technical Product Manager	Bạn đọc được PR để hiểu thay đổi ảnh hưởng gì, không phải để duyệt.

Definition of done — dấu hiệu bạn đã đạt

PR được phản hồi trong một ngày làm việc.
Mọi nhận xét đều có mức: blocking, nên, hay nit.
Lỗi lặp lại đã thành luật lint, không còn nhắc bằng miệng.
Hơn một người review được mỗi vùng code.

Đọc duyệt mã nguồn phục vụ ba mục đích, không phải một

Mục đích	Nghĩa là	Bị bỏ quên?
Bắt lỗi	Tìm vấn đề trước khi nó ra môi trường thật	Không — ai cũng nhớ
Lan truyền kiến thức	Người thứ hai hiểu vùng code đó	Có — và đây là giá trị lớn nhất về lâu dài
Giữ mã nguồn nhất quán	Code mới trông giống code cũ	Có

Khi chỉ nhìn mục đích thứ nhất, đội sẽ tối ưu cho tốc độ duyệt — duyệt nhanh, ít bình luận, xong việc. Điều đó bỏ mất hai mục đích còn lại, và hậu quả chỉ xuất hiện khi người viết đoạn code đó nghỉ việc.

Cách duyệt cho hiệu quả — người duyệt

Đọc mô tả trước, đọc code sau. Không hiểu nó định làm gì thì không duyệt được — và câu hỏi đầu tiên nên là "vì sao", không phải "như thế nào".
Phân biệt ba mức góp ý: chặn (phải sửa), nên sửa (không chặn), ghi chú (chỉ là ý kiến). Không phân mức thì mọi góp ý đều nặng như nhau và người viết không biết ưu tiên gì.
Góp ý về code, không về người. "Hàm này xử lý ca rỗng chưa?" thay vì "bạn quên ca rỗng".
Nói rõ khi bạn không chắc. "Mình không rành phần này, nhưng…" giúp người viết biết trọng lượng của góp ý.
Khen phần làm tốt. Nghe sáo nhưng có tác dụng thật: một buổi duyệt chỉ toàn lỗi sẽ khiến người ta viết phòng thủ thay vì viết tốt.

Về kích thước: thay đổi trên 400 dòng thì chất lượng duyệt giảm mạnh — người duyệt bắt đầu lướt. Nếu thay đổi lớn, chia nhỏ trước khi gửi duyệt.

Cách nhận duyệt — người viết

Đừng	Hãy	Vì sao
Gửi 2000 dòng một lần	Chia thành nhiều phần nhỏ	Trên 400 dòng thì người duyệt lướt
Mô tả trống	Viết rõ nó làm gì và vì sao	Người duyệt không đọc được ý định từ code
Tranh luận từng bình luận	Hỏi làm rõ, rồi quyết	Duyệt không phải cuộc thi
Im lặng khi không đồng ý	Nói rõ lý do và đánh đổi	Im lặng rồi làm theo tạo ra code không ai tin

Khi bất đồng không giải quyết được trong bình luận: gọi 5 phút. Một cuộc trao đổi dài 20 bình luận gần như luôn là cuộc gọi 5 phút bị kéo dài — và nó thường mang theo cảm xúc mà cuộc gọi không mang.

Vì sao vai trò của bạn cần cái này

Vai trò	Bạn dùng nó để
Engineer	Đây là nơi bạn học nhanh nhất, cả khi duyệt lẫn khi được duyệt
Team Lead	Duyệt là công cụ lan truyền kiến thức mạnh nhất bạn có — đừng để nó thành thủ tục
Product Owner	Hiểu vì sao thay đổi lớn cần chia nhỏ, kể cả khi nó làm chậm việc gộp
Người mới	Duyệt code của người khác là cách hiểu hệ thống nhanh hơn đọc tài liệu
Engineering Manager	Không vùng nào chỉ một người duyệt được — đó là rủi ro tổ chức

Hiccups thường gặp và cách xử lý

Hiccup	Vì sao xảy ra	Xử lý
Duyệt xong trong 2 phút, không bình luận gì	Coi duyệt là thủ tục chặn đường	Nếu không có gì để nói, ít nhất hỏi một câu về ý định — duyệt im lặng không lan truyền kiến thức
Chờ duyệt nhiều ngày	Không ai coi duyệt là việc ưu tiên	Duyệt trước khi bắt đầu việc mới; thời gian chờ duyệt là thời gian chết của cả đội
Bình luận nghe như chỉ trích	Viết vội, thiếu ngữ cảnh	Hướng vào code, phân mức rõ, và nói khi bạn không chắc
Cùng một góp ý lặp lại mãi	Chưa thành quy ước chung	Đưa vào tài liệu quy ước hoặc công cụ kiểm tự động — đừng để người nhắc lại mãi