Code Review Checklist: What I Look for After 10 Years of PRs \u2014 TXT1.ai

Ba năm trước, tôi đã phê duyệt một yêu cầu kéo khiến công ty tôi mất 47,000 USD doanh thu trong một cuối tuần. Mã nhìn có vẻ ổn. Các bài kiểm tra đã vượt qua. Logic là hợp lý. Nhưng tôi đã bỏ sót một điều tinh tế trong cách chúng tôi xử lý chuyển đổi múi giờ cho khách hàng ở châu Âu, và khi giờ mùa hè bắt đầu, hệ thống xử lý thanh toán của chúng tôi bắt đầu im lặng thất bại đối với 18% người dùng.

Vụ việc đó đã thay đổi cách tôi xem xét mã mãi mãi. Tôi là Sarah Chen, và tôi đã là một quản lý kỹ thuật cấp cao tại ba công ty SaaS khác nhau trong suốt thập kỷ qua, xem xét trung bình 15-20 yêu cầu kéo mỗi tuần. Khoảng 8,000 yêu cầu PR trong sự nghiệp của tôi, từ các bản sửa lỗi một dòng đến các tái cấu trúc kiến trúc lớn trải dài hơn 50 tệp. Tôi đã thấy mã xuất sắc mà lại gửi lỗi, và mã rối rắm mà hoạt động hoàn hảo trong sản xuất trong nhiều năm.

Điều tôi đã học được là việc xem xét mã không thực sự chỉ là tìm lỗi—mặc dù điều đó quan trọng. Nó là về việc xây dựng các hệ thống sống sót sau khi tiếp xúc với thực tế. Nó là về việc tạo ra mã mà lập trình viên tiếp theo (thường là bạn, sau sáu tháng) có thể hiểu và sửa đổi mà không sợ hãi. Và nó là về việc tìm ra những vấn đề vô hình chỉ lộ diện khi ứng dụng của bạn mở rộng, khi xảy ra các trường hợp biên, hoặc khi một khách hàng ở Úc làm điều gì đó mà bạn không bao giờ lường trước được.

Đây là danh sách kiểm tra của tôi. Không phải là lý thuyết từ sách giáo khoa, mà là thực tiễn đã được kiểm chứng mà tôi đã tinh chỉnh qua vô số sự cố sản xuất, các phiên gỡ lỗi muộn màng, và cảm giác chùng xuống khi bạn nhận ra một lỗi mà bạn đã phê duyệt hiện đang ảnh hưởng đến người dùng thực.

30 Giây Đầu Tiên: Những Gì Mô Tả PR Nói Với Tôi

Trước khi tôi nhìn vào một dòng mã, tôi dành 30 giây để đọc mô tả PR. Điều này có thể có vẻ không đáng kể, nhưng một mô tả viết kém thường là tín hiệu đỏ đầu tiên cho thấy mã có thể có vấn đề. Theo kinh nghiệm của tôi, các lập trình viên không thể diễn đạt rõ ràng những gì mã của họ thực hiện thường không suy nghĩ đầy đủ về việc triển khai.

Tôi tìm kiếm ba điều cụ thể: vấn đề đang được giải quyết, phương pháp đã thực hiện, và bất kỳ sự đánh đổi nào đã thực hiện. Một mô tả PR tốt đọc giống như một tài liệu thiết kế mini. "Sửa lỗi xác thực người dùng" không nói với tôi điều gì. "Sửa điều kiện đua trong việc làm mới token JWT đã khiến 0,3% người dùng bị đăng xuất bất ngờ trong các khoảng thời gian lưu lượng cao bằng cách triển khai một khóa phân tán sử dụng Redis" cho tôi biết lập trình viên hiểu sâu vấn đề.

Tôi cũng kiểm tra xem PR có kích thước phù hợp hay không. Quy tắc của tôi: nếu một PR tác động đến hơn 400 dòng mã (không tính các bài kiểm tra và các tệp được tạo), thì có khả năng nó quá lớn để xem xét một cách hiệu quả. Nghiên cứu từ các thực tiễn kỹ thuật của Google cho thấy hiệu quả xem xét giảm mạnh sau 200-400 dòng. Tôi thấy điều này là chính xác—sau khi xem xét khoảng 300 dòng, sự chú ý của tôi bắt đầu dao động, và tôi bắt đầu bỏ lỡ những vấn đề tinh tế.

Khi gặp một PR lớn, tôi yêu cầu lập trình viên phân nhỏ nó thành các phần nhỏ, hợp lý. Vâng, điều này mất thêm thời gian ngay từ đầu, nhưng nó ngăn chặn kịch bản mà tôi phê duyệt một PR 2,000 dòng vì tôi bị choáng ngợp và chỉ muốn hoàn thành nó. Tôi đã phê duyệt quá nhiều PR có vấn đề theo cách này trong những ngày đầu sự nghiệp của mình.

Mô tả cũng nên liên kết đến ngữ cảnh liên quan: vé hoặc vấn đề đang được giải quyết, bất kỳ tài liệu thiết kế nào, và các PR liên quan. Nếu tôi phải tìm kiếm qua Jira hoặc Slack để hiểu lý do cho sự thay đổi này, thì đó là sự cản trở làm chậm toàn bộ quy trình xem xét. Ở công ty hiện tại của tôi, chúng tôi đã đưa ra chính sách rằng các PR không có ngữ cảnh thích hợp sẽ không được xem xét cho đến khi mô tả được cập nhật. Quy tắc đơn giản này đã cải thiện đáng kể chất lượng xem xét của chúng tôi.

Lớp Logic: Liệu Điều Này Thực Sự Giải Quyết Vấn Đề Không?

Khi tôi hiểu được PR dự kiến sẽ làm gì, tôi xác minh rằng nó thực sự làm được điều đó. Điều này nghe có vẻ hiển nhiên, nhưng bạn sẽ ngạc nhiên về việc mã thường vượt qua bài kiểm tra nhưng không hoàn toàn giải quyết được vấn đề cơ bản. Tôi đã thấy các lập trình viên sửa chữa triệu chứng trong khi để mặc nguyên nguyên nhân gốc, tạo ra một quả bom hẹn giờ sẽ phát nổ sau này.

"Xem xét mã không thực sự chỉ là tìm lỗi—nó là về việc xây dựng các hệ thống mà sống sót sau khi tiếp xúc với thực tế và tạo ra mã mà lập trình viên tiếp theo có thể hiểu mà không sợ hãi."

Tôi bắt đầu bằng việc tưởng tượng theo con đường hạnh phúc. Nếu đây là một tính năng mới để xử lý hoàn tiền, tôi hình dung một khách hàng yêu cầu hoàn tiền và theo dõi con đường mã từ điểm cuối API qua logic doanh nghiệp tới cập nhật cơ sở dữ liệu. Liệu mỗi bước có hợp lý không? Chúng tôi có xử lý dữ liệu đúng cách không? Chúng tôi có cập nhật tất cả các bản ghi cần thiết không?

Nhưng con đường hạnh phúc thì dễ dàng. Điều phân biệt mã tốt với mã sẵn sàng cho sản xuất là cách mà nó xử lý các con đường không hạnh phúc. Điều gì sẽ xảy ra nếu cổng thanh toán bị ngừng hoạt động? Điều gì sẽ xảy ra nếu số tiền hoàn lại là âm? Điều gì sẽ xảy ra nếu người dùng yêu cầu hoàn tiền cho một đơn hàng đã được hoàn tiền? Tôi đã học được rằng cần phải gần như cẩn thận đến mức mắc chứng hoang tưởng về các trường hợp biên vì môi trường sản xuất là những cỗ máy hỗn loạn sẽ tìm thấy mọi trường hợp biên mà bạn không xem xét.

Tôi tìm kiếm các mẫu lập trình phòng ngừa: kiểm tra null, xác thực đầu vào, xử lý điều kiện ranh giới. Nhưng tôi cũng tìm kiếm việc kỹ thuật hóa quá mức. Không phải mọi hàm đều cần phải xử lý mọi trường hợp biên có thể xảy ra. Nếu một phương thức riêng chỉ được gọi từ một nơi với đầu vào đã được xác thực, việc thêm xác thực thừa chỉ là tiếng ồn. Chìa khóa là hiểu hợp đồng của mỗi hàm và đảm bảo rằng nó được tuân thủ ở các biên.

Một mẫu mà tôi đã thấy gây ra vấn đề lặp đi lặp lại là "cập nhật lạc quan." Một lập trình viên giả định rằng một thao tác sẽ thành công và cập nhật giao diện người dùng hoặc trạng thái cơ sở dữ liệu trước khi xác nhận thành công. Điều này hoạt động 99% thời gian, nhưng 1% đó tạo ra những lỗi cực kỳ khó hiểu nơi trạng thái hệ thống trở nên không nhất quán. Tôi luôn đánh dấu những mẫu này và hỏi: điều gì xảy ra nếu thao tác này thất bại giữa chừng?

Lớp Dữ Liệu: Theo Dõi Luồng Thông Tin

Lỗi dữ liệu là loại lỗi tồi tệ nhất vì chúng thường im lặng và tích lũy. Một lỗi logic có thể gây sập ứng dụng của bạn ngay lập tức, nhưng một lỗi dữ liệu có thể làm hỏng cơ sở dữ liệu của bạn từ từ trong suốt nhiều tuần, và đến khi bạn nhận ra, bạn có hàng ngàn bản ghi xấu và không có cách nào sạch để sửa chữa chúng.

Khi xem xét mã chạm vào dữ liệu, tôi theo dõi toàn bộ vòng đời của dữ liệu đó. Nó đến từ đâu? Nó được xác thực như thế nào? Nó được biến đổi ra sao? Nó được lưu trữ ở đâu? Ai có thể truy cập nó? Điều này đặc biệt quan trọng cho nội dung do người dùng tạo hoặc dữ liệu tài chính nơi mà các lỗi có hậu quả thực sự.

Tôi đặc biệt chú ý đến các chuyển đổi kiểu dữ liệu. Một lần, tôi đã phê duyệt mã chuyển đổi giá từ số thực thành số nguyên cho một phép tính, sau đó trở lại số thực. Có vẻ vô hại. Nhưng do vấn đề chính xác của số thực, các giá như $10.99 thỉnh thoảng sẽ trở thành $10.98 hoặc $11.00 sau khi quay vòng. Chúng tôi đã không phát hiện ra điều này cho đến khi khách hàng bắt đầu phàn nàn về các khoản thanh toán không chính xác. Bây giờ tôi kiểm tra mọi chuyển đổi kiểu, đặc biệt là liên quan đến tiền bạc hoặc phép đo.

Các di chuyển cơ sở dữ liệu là một lĩnh vực khác mà tôi rất thận trọng. Tôi tìm kiếm một số điều: Liệu việc di chuyển có thể đảo ngược không? Điều gì xảy ra nếu nó thất bại giữa chừng? Nó sẽ mất bao lâu trên cơ sở dữ liệu sản xuất của chúng tôi? Tôi đã thấy những lần di chuyển hoạt động tốt trên một cơ sở dữ liệu phát triển với 1,000 bản ghi nhưng khóa bảng suốt 20 phút trên một cơ sở dữ liệu sản xuất với 50 triệu bản ghi, làm sập toàn bộ ứng dụng.

Đối với bất kỳ mã nào sửa đổi dữ liệu hiện có, tôi hỏi: có kế hoạch sao lưu không? Chúng tôi có thể quay lại nếu có điều gì sai xảy ra không? Tại công ty trước đây của tôi, chúng tôi có một chính sách rằng bất kỳ di chuyển dữ liệu nào ảnh hưởng đến hơn 10,000 bản ghi đều yêu cầu một lần chạy thử trên một ảnh chụp sản xuất và một thủ tục quay lại được ghi lại trong PR. Điều này đã cứu chúng tôi nhiều lần.

Tôi cũng nhìn vào các mẫu truy cập dữ liệu. Mã này có đang thực hiện các truy vấn N+1 không? Nó có tải toàn bộ bộ sưu tập vào bộ nhớ khi chỉ cần một vài trường không? Những vấn đề hiệu suất này có thể không rõ ràng trong phát triển nhưng trở thành rất quan trọng khi mở rộng. Tôi đã từng xem xét mã có vẻ tốt nhưng sẽ khiến CPU cơ sở dữ liệu của chúng tôi tăng lên 100% nếu nó được gửi đến sản xuất, chỉ vì nó đã