Ba năm trước, tôi đã phê duyệt một yêu cầu kéo mà khiến công ty tôi mất 47.000 đô la doanh thu chỉ trong một cuối tuần. Mã nhìn thì ổn. Các bài kiểm tra đã vượt qua. Logic có vẻ hợp lý. Nhưng tôi đã bỏ lỡ điều gì đó tinh vi trong cách chúng tôi xử lý chuyển đổi múi giờ cho hệ thống thanh toán đăng ký của mình, và khách hàng ở một số vùng bị tính phí hai lần.
💡 Những Điều Cần Nhớ
- 30 Giây Đầu Tiên: Những Gì Mô Tả PR Nói Với Bạn
- Kích Thước Quan Trọng: Quy Tắc 400 Dòng
- Tầng Logic Kinh Doanh: Nơi Ẩn Chứa Nhiều Lỗi Nhất
- Xử Lý Lỗi: Sự Khác Biệt Giữa Mã Tốt và Vượt Trội
Sự cố đó đã thay đổi cách tôi đánh giá mã mãi mãi. Tôi là Sarah Chen, và tôi đã làm quản lý kỹ thuật cao cấp 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 PR trong sự nghiệp của tôi. Tôi đã thấy mã xuất sắc nhưng lại chứa lỗi, và mã rối rắm nhưng chạy hoàn hảo trong sản xuất trong nhiều năm. Tôi đã học được rằng đánh giá mã không chỉ đơn thuần là tìm lỗi cú pháp - công cụ kiểm tra của bạn làm điều đó. Nó liên quan đến việc phát hiện các vấn đề vô hình mà chỉ kinh nghiệm mới có thể tiết lộ.
Bài viết này là danh sách kiểm tra mà tôi ước mình đã có khi mới bắt đầu. Nó không đầy đủ - không có danh sách kiểm tra nào có thể - nhưng nó đại diện cho những mẫu mà tôi đã học để nhận diện, những câu hỏi mà tôi đã tự đào tạo để hỏi, và những mô hình tư duy đã cứu đội ngũ của tôi khỏi vô số sự cố trong sản xuất.
30 Giây Đầu Tiên: Những Gì Mô Tả PR Nói Với Bạn
Trước khi tôi xem một dòng mã nào, tôi dành 30 giây để đọc mô tả PR. Điều này có thể có vẻ ngược lại, nhưng tôi đã thấy rằng 60% các PR có vấn đề tự tiết lộ ngay lập tức thông qua việc giao tiếp kém. Một mô tả PR tốt trả lời ba câu hỏi: cái gì đã thay đổi, tại sao lại thay đổi, và những điều gì có thể sai.
Khi tôi thấy mô tả nói "cố định lỗi" hay "cập nhật thành phần," tôi ngay lập tức biết rằng đánh giá này sẽ kéo dài hơn. Tác giả chưa nghĩ đến những hệ quả của những thay đổi của họ, điều này có nghĩa là tôi cần làm công việc đó cho họ. Ngược lại, khi tôi thấy một mô tả nói "Đã tái cấu trúc xác thực người dùng để sử dụng JWT thay vì cookie phiên. Điều này giảm tải cơ sở dữ liệu xuống 40% trong giờ cao điểm nhưng yêu cầu các khách hàng xử lý việc làm mới token. Rủi ro: các phiên bản ứng dụng di động hiện tại (< 2.3) sẽ cần phải xác thực lại," tôi biết tác giả đã làm bài tập về nhà của họ.
Tôi tìm kiếm các chỉ số cụ thể trong mô tả. Không phải những cải tiến mơ hồ, mà là số liệu thực tế. "Nâng cao hiệu suất" thì không có nghĩa gì. "Giảm thời gian phản hồi API từ 340ms xuống 180ms cho điểm cuối hồ sơ người dùng dưới 1000 yêu cầu đồng thời" cho tôi biết tác giả đã đo lường công việc của họ và hiểu được tác động. Theo kinh nghiệm của tôi, các nhà phát triển bao gồm các chỉ số trong các mô tả PR của họ viết mã có suy nghĩ hơn trong tổng thể.
Mô tả cũng nên liên kết đến bối cảnh liên quan - vé, tài liệu thiết kế, chuỗi Slack nơi cách tiếp cận được thảo luận. Nếu tôi đang xem xét một PR một cách tách biệt, không hiểu bối cảnh rộng hơn, tôi sẽ bỏ lỡ nhiều điều. Tôi đã từng phê duyệt một "tái cấu trúc đơn giản" mà vô tình làm hỏng một sự tích hợp quan trọng vì tôi không biết về một phụ thuộc không được tài liệu ở bất kỳ đâu ngoại trừ trong một chuỗi email ba tháng tuổi.
Các tín hiệu đỏ trong mô tả PR bao gồm: không có mô tả nào cả, các mô tả dài hơn sự thay đổi mã (thường chỉ ra việc kỹ thuật hóa quá mức), các mô tả nói "WIP" hay "dự thảo" nhưng PR được đánh dấu là sẵn sàng cho đánh giá, và các mô tả bao gồm các cụm từ như "không chắc đây là cách tiếp cận đúng" (vậy tại sao bạn lại yêu cầu tôi đánh giá điều này?).
Kích Thước Quan Trọng: Quy Tắc 400 Dòng
Tôi có một quy tắc cứng: Tôi không đánh giá kỹ lưỡng các PR lớn hơn 400 dòng thay đổi thực tế (không bao gồm mã được tạo ra, các tệp package-lock, và các mẫu thử nghiệm). Điều này không phải ngẫu nhiên. Nghiên cứu từ Cisco và SmartBear cho thấy hiệu quả đánh giá mã giảm đáng kể sau 400 dòng, và kinh nghiệm của tôi cũng xác nhận điều này. Sau khi xem xét khoảng 200 dòng mã, não tôi bắt đầu bỏ qua các chi tiết. Đến 400 dòng, tôi về cơ bản chỉ lướt qua.
"Đánh giá mã không phải là tìm lỗi cú pháp - công cụ kiểm tra của bạn làm điều đó. Nó liên quan đến việc phát hiện những vấn đề vô hình mà chỉ kinh nghiệm mới có thể tiết lộ."
Khi ai đó gửi một PR dài 1.200 dòng, tôi yêu cầu họ chia nhỏ nó. Tôi không quan tâm nếu tất cả đều liên quan. Các PR lớn là nơi các lỗi ẩn náu. Tôi đã thấy các lỗ hổng bảo mật quan trọng lọt qua trong các PR tái cấu trúc lớn vì các đánh giá viên cảm thấy mệt mỏi và ngừng chú ý khoảng dòng 600. Lỗ hổng đó nằm ở dòng 847.
Tất nhiên có những trường hợp ngoại lệ. Đôi khi bạn cần di chuyển các tệp quanh, hoặc cập nhật mã được tạo ra, hoặc thực hiện thay đổi toàn diện để đáp ứng một tiêu chuẩn kiểm tra mới. Trong những trường hợp đó, tôi yêu cầu tác giả tách biệt các thay đổi cơ khí khỏi các thay đổi logic. Gửi các di chuyển tệp trong một PR, các thay đổi logic thực tế trong PR khác. Điều này làm cho cả hai PR dễ dàng hơn để xem xét và dễ dàng hơn để quay ngược lại nếu có điều gì đó sai.
Tôi cũng chú ý đến tỷ lệ mã được thêm vào so với mã bị xóa. Theo kinh nghiệm của tôi, các PR tốt nhất có tính lượng dòng ròng âm - họ đạt được điều gì đó trong khi làm giảm quy mô mã. Khi tôi thấy một PR thêm 500 dòng và xóa 50, tôi cảm thấy nghi ngờ. Chúng ta có đang làm phức tạp thêm không? Chúng ta có đang sao chép chức năng hiện có không? Tác giả có nhận thức về những gì đã có trong mã không?
Vấn đề kích thước cũng mở rộng đến các tệp cá nhân. Nếu một PR tác động đến 30 tệp khác nhau, ngay cả khi tổng số dòng là hợp lý, đó cũng là một tín hiệu đỏ. Nó gợi ý rằng thay đổi có thể thiếu phạm vi rõ ràng hoặc mã có các vấn đề liên kết. Dù sao thì, nó xứng đáng được xem xét thêm.
Tầng Logic Kinh Doanh: Nơi Ẩn Chứa Nhiều Lỗi Nhất
Sau một thập kỷ đánh giá mã, tôi có thể nói cho bạn biết nơi lỗi ẩn náu: trong tầng logic kinh doanh, đặc biệt là trong các trường hợp biên và chuyển trạng thái. Không phải trong các thành phần giao diện. Không phải trong các truy vấn cơ sở dữ liệu. Mà là trong mã thực hiện các quy tắc kinh doanh thực sự của bạn.
| Chất Lượng Mô Tả PR | Nó Nói Gì | Thời Gian Đánh Giá | Mức Độ Rủi Ro |
|---|---|---|---|
| Kém | "cố định lỗi" hoặc "cập nhật thành phần" | 2-3x lâu hơn | Cao |
| Tốt Đủ | Cơ bản cái gì và tại sao, không có phân tích rủi ro | Bình thường | Trung bình |
| Tốt | Rõ ràng cái gì, tại sao, và các rủi ro tiềm năng đã được xác định | Hiệu quả | Thấp |
| Xuất Sắc | Bối cảnh toàn diện, thảo luận về trade-offs, các trường hợp biên được ghi chú | Nhanh | Rất Thấp |
Khi tôi đánh giá logic kinh doanh, tôi đang tìm kiếm các giả định. Mỗi giả định là một lỗi tiềm năng. Tôi đã từng đánh giá mã cho một hệ thống tính toán giảm giá mà giả định tất cả các giảm giá là phần trăm. Mã chạy hoàn hảo cho đến khi tiếp thị muốn cung cấp một chương trình khuyến mãi "$10 giảm giá". Hệ thống đã bị sập vì ai đó đã chia cho số tiền giảm giá, và chia cho 10 khi mà bạn mong đợi một số giữa 0 và 1 sản sinh ra kết quả rất sai.
Tôi tự hỏi: điều gì xảy ra ở các ranh giới? Điều gì xảy ra nếu đầu vào là 0? Điều gì xảy ra nếu nó là số âm? Điều gì xảy ra nếu nó là null? Điều gì xảy ra nếu đó là chuỗi rỗng so với chuỗi null? Điều gì xảy ra nếu mảng là rỗng? Điều gì xảy ra nếu người dùng không có quyền? Điều gì xảy ra nếu họ có tất cả quyền? Điều gì xảy ra nếu cơ sở dữ liệu không trả về kết quả nào? Điều gì xảy ra nếu nó trả về một triệu kết quả?
Tôi tìm kiếm các số ma trong logic kinh doanh. Khi tôi thấy if (user.loginAttempts > 5), tôi hỏi: vì sao là 5? Có được tài liệu không? Có thể cấu hình được không? Điều gì xảy ra nếu chúng ta muốn thay đổi nó thành 3 cho một số loại người dùng nhất định? Các số ma trong logic kinh doanh là nợ kỹ thuật đang chờ xảy ra.
Các máy trạng thái là một nguồn phổ biến khác của lỗi. Khi tôi thấy mã quản lý các chuyển trạng thái - trạng thái đơn hàng, vòng đời người dùng, các giai đoạn quy trình làm việc - tôi vẽ sơ đồ trạng thái trong đầu. Tất cả các chuyển tiếp đều được tính đến chưa? Bạn có thể rơi vào trạng thái không hợp lệ không? Tôi đã từng bắt được một lỗi mà người dùng có thể đồng thời ở trạng thái "hoạt động" và "tạm ngưng" vì mã thiết lập một trạng thái không kiểm tra trạng thái khác.
🛠 Khám Phá Các Công Cụ Của Chúng Tôi
Tôi cũng theo dõi logic kinh doanh nằm rải rác qua nhiều lớp khác nhau. Nếu tôi thấy xác thực trong giao diện người dùng, thêm xác thực trong API, và lại thêm xác thực trong lớp cơ sở dữ liệu, tôi biết chúng ta sẽ có những điểm không nhất quán. Các quy tắc kinh doanh nên sống ở một nơi, và nơi đó nên rõ ràng và dễ hiểu.