Guidelines cho Code Review

Bài viết gốc: https://engineeringblog.yelp.com/2017/11/code-review-guidelines.html#when-reviewing-code

Jonathan Maltz, Kỹ Sư Phần Mềm 20 tháng 11, 2017

Chúng tôi đặc biệt coi trọng việc review code và cho rằng đây là yếu tố then chốt để xây dựng một tổ chức kỹ thuật hoạt động hiệu quả cao.

Review code tạo ra những đoạn code chất lượng cao hơn và được nhiều người hiểu hơn. Nó còn giúp các kỹ sư học hỏi từ đồng nghiệp, thực hành mentorship, và tham gia vào các cuộc đối thoại, thảo luận cởi mở về những gì họ xây dựng. Lợi ích của review code phù hợp với giá trị Hợp tác tốt với người khác của Yelp và hỗ trợ văn hóa không ngừng dạy và học của chúng tôi.

Khi tổ chức tiếp tục phát triển, có những quy tắc nhất định giúp việc review code trở nên hiệu quả hơn và tránh trở thành điểm tắc nghẽn. Chúng tôi đã tuân theo các nguyên tắc này trong nhiều năm và muốn chia sẻ chúng với cộng đồng rộng lớn hơn vì chúng thực sự hiệu quả với chúng tôi.

Jason Fennell, Phó Chủ Tịch Cấp Cao Kỹ Thuật

Hướng Dẫn Review Code

Các hướng dẫn này xuất phát từ những gì review code cần đạt được. Chúng tôi không thể đặt ra các hướng dẫn áp dụng cho mọi tình huống, vì vậy việc luôn ghi nhớ các mục tiêu này sẽ giúp bạn tuân thủ “tinh thần” của review code ngay cả khi gặp tình huống chưa được đề cập.

Review code nên:

• Xác minh rằng code là giải pháp đúng đắn và hiệu quả cho các yêu cầu hiện tại
• Đảm bảo code của bạn có thể bảo trì được
• Tăng cường kiến thức chung về codebase
• Nâng cao kỹ năng của nhóm thông qua phản hồi thường xuyên
• Không trở thành gánh nặng tốn kém thời gian của developer

Hướng Dẫn

Khi Đăng Một Code Review

Chọn Đúng Người Review

Bạn nên chọn những người review có thể xác nhận rằng code của bạn đúng, có kiến trúc tốt, và tuân thủ các quy ước trong một khoảng thời gian hợp lý. Giao tiếp là chìa khóa để tránh bị chặn trong quá trình review. Nếu bạn thấy người review đang làm chậm quá trình, hãy phối hợp với họ để tìm người review phù hợp hơn hoặc xác định lịch trình phù hợp cho tất cả mọi người.

Luôn Có Một Người Review Chính

Người review chính chịu trách nhiệm tổng thể cho toàn bộ quá trình review code. Họ chịu trách nhiệm về code cuối cùng ngang bằng với người viết code. Bạn nên luôn xác định rõ ràng một người review chính để mọi người biết ai là người chịu trách nhiệm cuối cùng.

Không được ship code khi chưa có sự phê duyệt từ người review chính, trừ khi bạn đang gặp tình huống khẩn cấp và người review chính không thể liên lạc được.

Xác Định Rõ Trách Nhiệm Của Từng Người Review

Để tránh trùng lặp khi nhiều người cùng review một đoạn code, hãy xác định rõ ràng phần nào mỗi người chịu trách nhiệm và ai là người review chính. Điều này cho phép mỗi người tập trung vào lĩnh vực chuyên môn của mình (chẳng hạn như các DBA) và giữ cho các cuộc thảo luận có thể quản lý được.

Chuẩn Bị Code Để Review

Giao Tiếp Là Chìa Khóa

Hãy cung cấp cho người review bối cảnh về thay đổi của bạn. Lý tưởng nhất là bạn nên trao đổi với họ trước bất kỳ review không trivial nào, hoặc ghi lại các thay đổi bạn đang thực hiện trong phần mô tả của review. Hãy đảm bảo tóm tắt thay đổi bạn đang thực hiện, lý do thực hiện những thay đổi đó, và liên kết đến ticket hoặc spec để cung cấp thêm bối cảnh. Như đã đề cập trước đó, hãy thiết lập lịch trình với người review để họ biết bạn cần phản hồi nhanh như thế nào (xem: “Nhanh hơn là tốt hơn” để biết hướng dẫn tổng quát).

Đôi khi cách hiệu quả nhất để giải quyết bất đồng là trực tiếp trò chuyện (ví dụ: ngoại tuyến hoặc qua chat). Nếu bạn thấy mình đang có những cuộc thảo luận dài trong review code, hãy liên hệ với người review để giải quyết bất đồng kịp thời.

Nếu bạn có các cuộc thảo luận ngoại tuyến, hãy tóm tắt cuộc thảo luận và kế hoạch hành động trong code review để tham khảo sau này và cung cấp bối cảnh cho các người review khác.

Nhỏ Hơn Là Tốt Hơn

Giữ cho code review của bạn nhỏ gọn để bạn có thể lặp lại nhanh hơn và chính xác hơn. Nhìn chung, các thay đổi code nhỏ hơn cũng dễ kiểm tra và xác minh tính ổn định hơn.

Để giúp giữ cho code review nhỏ gọn, hãy tách riêng review thay đổi logic với review thay đổi style code. Nếu bạn đã thay đổi cả hai, hãy submit thay đổi style code như một branch riêng và sau đó tiếp tục với branch thay đổi logic.

Làm Cho Code Dễ Review

Code nên chứa cả comment cấp cao và comment nội tuyến. Comment cấp cao giải thích cách các thành phần kết hợp với nhau và cách xử lý các trường hợp ngoại lệ, trong khi comment nội tuyến mô tả lý do code có hình dạng hiện tại. Điều này sẽ giúp code dễ hiểu hơn cho người bảo trì và người review.

Nếu bạn cảm thấy code review vẫn khó hiểu sau khi đã thêm tài liệu, bạn nên chỉ định điểm bắt đầu cho người review và chi tiết các phần code có thể bỏ qua.

Đọc Lại Diff Trước Khi Gửi Đi Review

Trước khi submit để review, bạn nên tự review diff của mình để tìm lỗi. Hãy cố gắng nhìn qua con mắt của người chưa bao giờ nhìn thấy code trước đây. Tìm kiếm các quyết định có thể gây nhầm lẫn và có thể cần giải thích trước. Nếu bạn cảm thấy code quá khó hiểu, bạn có thể muốn tinh chỉnh thêm trước khi gửi đi review. Phát hiện sớm các vấn đề này sẽ tiết kiệm thời gian cho cả bạn và người review.

Trong Quá Trình Review Code

Tránh Thay Đổi Lớn Trong Khi Đang Review

Các thay đổi lớn ở giữa quá trình review về cơ bản sẽ reset toàn bộ quá trình review. Nếu bạn cần thực hiện các thay đổi lớn sau khi đã submit review, bạn có thể muốn ship review hiện tại và tiếp tục với các thay đổi bổ sung. Nếu bạn cần thực hiện các thay đổi lớn sau khi bắt đầu quá trình review, hãy đảm bảo thông báo điều này cho người review càng sớm càng tốt.

Hoàn toàn chấp nhận được khi tiến hành review “nháp” để xin ý kiến sơ bộ, nhưng hãy đảm bảo thông báo rõ ràng điều này với người review để tránh nhầm lẫn. Sau đó bạn sẽ muốn thông báo với người review khi review của bạn đã rời khỏi trạng thái “nháp” hoặc mở một code review mới.

Phản Hồi Tất Cả Các Phản Hồi Review Có Thể Thực Hiện

Bạn nên hiểu từng phản hồi từ người review và trả lời từng phần có thể thực hiện. Ngay cả khi bạn không thực hiện phản hồi của họ, hãy trả lời và giải thích lý do. Nếu có điều gì đó bạn không hiểu, hãy đặt câu hỏi trong hoặc ngoài code review. Xem “Giao tiếp là chìa khóa” để biết thêm thông tin.

Review Code Là Thảo Luận, Không Phải Chỉ Đạo

Bạn có thể xem hầu hết các phản hồi review code như là gợi ý hơn là mệnh lệnh. Hoàn toàn chấp nhận được khi không đồng ý với phản hồi của người review, nhưng bạn cần giải thích lý do và cho họ cơ hội phản hồi.

Khi Review Code

Bạn Chịu Trách Nhiệm Về Code Mà Bạn Review

Bạn chịu trách nhiệm ngang bằng với người viết code về những gì được ship. Bạn có trách nhiệm đảm bảo rằng code đúng đắn, có kiến trúc tốt, bảo mật và có thể bảo trì. Nếu bạn không tự tin rằng code đáp ứng các tiêu chuẩn này, hãy nhờ đồng nghiệp giúp hoàn thành code review.

Không bao giờ cho “ship” nếu bạn không tự tin rằng code đáp ứng các tiêu chuẩn này.

Thời Gian Là Vàng Bạc

Bạn có thể trả lại code review cho người submit càng nhanh càng tốt. Lý tưởng nhất là code review nên được trả lại trong vòng 24 giờ để duy trì đà tiến của dự án. Điều này rõ ràng dễ thực hiện hơn với các code review nhỏ hơn (xem “Nhỏ hơn là tốt hơn” để biết thêm). Đối với các code review quy mô lớn hơn, kỳ vọng nên được thảo luận giữa bạn và người được review.

Hãy nhớ rằng toàn bộ code review không cần phải hoàn thành trong một lần ngồi. Nếu review lớn, hãy review từng phần code một lần và thông báo tiến độ của bạn. Không bao giờ ship code cho đến khi bạn đã review tất cả.

Review Về Tính Đúng Đắn

Khi review code, bạn phải đảm bảo rằng nó đúng đắn. Kiểm tra xem code có không có bug, giải quyết đúng vấn đề đặt ra và xử lý các edge case một cách thích hợp. Nếu bạn đang xử lý việc serialize/deserialize dữ liệu, hãy kiểm tra xem code có an toàn khi rollback và roll-forward không.

Review Về Logic Cốt Lõi Và Kiến Trúc, Không Chỉ Về Style

Hãy suy nghĩ kỹ về kiến trúc của code. Bạn phải có thể hiểu từng phần và cách chúng kết hợp với nhau. Xác nhận rằng logic của từng thành phần hiệu quả và kiến trúc linh hoạt nhưng không bị over-engineer.

Khi có nghi ngờ, hãy ưu tiên khả năng đọc và bảo trì.

Review Về Độ Phủ Test Đầy Đủ

Nói chung, tất cả code trong codebase đều nên được kiểm tra. Dành thời gian review chiến lược testing để đảm bảo tất cả code được kiểm tra kỹ lưỡng. Điều này có thể bao gồm unit test, integration test, regression test, v.v. Đảm bảo rằng các unit test được cô lập tốt và không có các dependency không cần thiết.

Cách Giao Tiếp Trong Code Review

Liên Kết Đến Hướng Dẫn Style Liên Quan

Nếu bạn chỉ ra style cần thay đổi để tuân thủ hướng dẫn style của nhóm, hãy liên kết đến tài liệu liên quan nêu rõ điều này. Nếu bạn đang đánh dấu thay đổi style không được đề cập trong hướng dẫn của nhóm, hãy suy nghĩ xem liệu nó có nên được thêm vào hướng dẫn không.

Nếu bạn thấy mình thường xuyên nhận xét về style, bạn nên tự động hóa kiểm tra codestyle thông qua pre-commit hook. Điều này sẽ cho phép bạn tập trung vào review code về tính đúng đắn hơn là style.

Giao Tiếp Là Chìa Khóa Và Review Code Là Thảo Luận, Không Phải Chỉ Đạo

Dù bạn đang review code hay có code được review, giao tiếp là yếu tố then chốt và cả hai bên cần nhận thức rằng phản hồi là gợi ý mở để thảo luận. Điều này có thể đòi hỏi một số thỏa hiệp và lựa chọn kiến trúc. Tuy nhiên, với tư cách là người review, bạn không nên cho code “ship” nếu bạn chưa hài lòng với cách xử lý một vấn đề còn mở.

Hãy nhớ rằng công việc của bạn với tư cách người review là thúc đẩy thảo luận, vì vậy hãy khuyến khích giao tiếp cởi mở cả online lẫn offline.

Danh Sách Kiểm Tra

Khi Đăng Một Code Review

Truyền đạt bối cảnh và yêu cầu cho người review

• [ ] Xác định người/những người tốt nhất để review thay đổi của bạn
• [ ] Truyền đạt thay đổi và mục đích của nó cho người review
• [ ] Thiết lập lịch trình với tất cả người review nếu bạn cần ship trước một ngày cụ thể
• [ ] Xác định người review chính
• [ ] Xác định trách nhiệm của từng người review

Đọc kỹ code trước khi xuất bản

• [ ] Đọc lại diff của bạn
• [ ] Đảm bảo diff thể hiện rõ ràng các thay đổi của bạn
• [ ] Code review của bạn có thể chia nhỏ thành các phần nhỏ hơn không?
• [ ] Đảm bảo code của bạn dễ theo dõi cho người review
• [ ] Kiểm tra style code
• [ ] Cung cấp tài liệu liên quan dễ dàng cho người review
• [ ] Xác nhận rằng người review của bạn biết về bất kỳ thay đổi lớn nào (nếu có) bạn dự định thực hiện trong quá trình review

Thảo luận phản hồi

• [ ] Phản hồi tất cả các nhận xét review code. Thảo luận về bất kỳ phản hồi nào bạn không đồng ý.

Khi Thực Hiện Code Review

Hiểu code

• [ ] Đảm bảo bạn hiểu hoàn toàn code
• [ ] Đánh giá tất cả các đánh đổi về kiến trúc

Kiểm tra chất lượng code

• [ ] Xác minh tính đúng đắn của code
• [ ] Kiểm tra logic cốt lõi được tổ chức tốt và hiệu quả
• [ ] Code có tính tổng quát đúng mức cần thiết, không tổng quát hơn mức cần thiết không?
• [ ] Đảm bảo code có thể bảo trì được
• [ ] Đảm bảo tính nhất quán về style với phần còn lại của codebase

Xác minh code được kiểm tra kỹ lưỡng

• [ ] Xác nhận độ phủ test đầy đủ
• [ ] Kiểm tra các test có đúng dependency và đang kiểm tra đúng thứ không

Đảm bảo code an toàn để deploy

• [ ] Hỏi xem code có tương thích xuôi/ngược không. Đặc biệt, hãy chú ý nếu code đang thay đổi việc serialize/deserialize của một thứ gì đó.
• [ ] Chạy thử kịch bản roll-back để kiểm tra tính an toàn khi rollback
• [ ] Kiểm tra các lỗ hổng bảo mật và liên hệ với nhóm bảo mật nếu bạn không chắc chắn
• [ ] Trả lời: Nếu code này bị lỗi lúc 3 giờ sáng và bạn được gọi để chẩn đoán và sửa lỗi, bạn có thể làm được không?