Review code cần phải đọc
(hauleth.dev)- Review code không phải là một thủ tục hình thức trước khi triển khai, mà là quá trình chuyển trách nhiệm về sự cố, vấn đề bảo mật và việc xóa dữ liệu từ trách nhiệm của một cá nhân sang trách nhiệm của cả nhóm
- Các câu trả lời như evals tốt hơn, test tốt hơn, feature flag, guardrail và khả năng quan sát có thể là nỗ lực nhằm giảm bớt sự bất an khi triển khai đoạn code chưa được đọc, nhưng bị phê phán là cách tiếp cận đã bỏ lỡ mục tiêu phân tán trách nhiệm của review
- Yêu cầu phê duyệt mà không đọc cũng giống như bắt con người bấm nút mà không cần suy nghĩ, dẫn tới lời châm biếm button roulette: merge ngẫu nhiên một PR bất kỳ miễn là CI đều xanh
- Review code khiến nhiều thành viên trong nhóm nhìn vào các phần khác nhau của một codebase lớn, từ đó giảm bus factor và đóng vai trò như một cơ chế học tập để thành viên mới làm quen với code cũng như văn hóa code
- Nếu muốn ép triển khai code chưa được đọc, thì cần có văn bản miễn trừ trách nhiệm đối với bug, vấn đề bảo mật, downtime...; và kết luận là gần như không ai dễ dàng chấp nhận đưa ra sự miễn trừ đó
Mục đích của review là phân tán trách nhiệm, không phải phê duyệt
- Điểm khởi đầu là câu hỏi trong bài viết của Charity Majors “AI enthusiasts are in a race against time, AI skeptics are in a race against entropy”: “Cần những gì để bạn cảm thấy thoải mái khi triển khai lên production một đoạn code mà chưa hề đọc?”
- Những câu trả lời như evals tốt hơn, test, feature flag, guardrail, khả năng quan sát, tách biệt phụ thuộc, thu nhỏ blast radius, hay bắt đầu từ các thay đổi nhỏ ngoài critical path được xem là đã né tránh trọng tâm của review
- Mục đích của review là không để gánh nặng về sự cố, vấn đề bảo mật hay việc xóa dữ liệu ngoài ý muốn dồn lên một cá nhân, mà chia sẻ nó thành trách nhiệm của cả nhóm gồm tác giả và reviewer
- Nếu yêu cầu “phê duyệt mà không đọc”, thì lý do để bắt con người bấm nút thủ công cũng trở nên yếu đi, và từ đó xuất hiện lời châm biếm
button roulette: ngẫu nhiên merge một trong các PR được phân công miễn là toàn bộ CI đều xanh
Review không đọc sẽ đánh mất điều gì
- Review code buộc các thành viên trong nhóm phải nhìn vào những phần khác nhau của codebase, qua đó bù đắp cho thực tế rằng với một hệ thống quá lớn, không ai có thể luôn hiểu mọi phần của nó
- Quá trình review giúp giảm bus factor, đồng thời giúp nhiều thành viên trong nhóm trở nên quen thuộc hơn với codebase và văn hóa code
- Nếu mọi người đều phê duyệt mà không đọc, thì hiệu quả học tập này sẽ biến mất, bus factor không chỉ tăng lên 1 mà còn phát sinh vấn đề bị externalize cho bên thứ ba
- Kết luận được tóm lại là: để chấp nhận yêu cầu triển khai lên production đoạn code chưa được đọc, người đưa ra chỉ thị đó phải cung cấp văn bản miễn trừ trách nhiệm đối với bug, vấn đề bảo mật, downtime...
1 bình luận
Ý kiến trên Lobste.rs
Cảm thấy rất phản cảm với lập luận cho rằng mục đích của review là phân tán trách nhiệm
Nếu đoạn code đã merge vào main làm hỏng production thì đó là trách nhiệm của người viết, không phải của tôi hay của cả nhóm
Đó là công việc mang chữ ký của chính mình với tư cách một người làm nghề chuyên nghiệp; nếu một bản thiết kế cầu được đóng dấu bằng giấy phép P.E. ngành xây dựng dân dụng mà gây chết người thì đó cũng là công việc và trách nhiệm của chính người đó
Trách nhiệm của cả nhóm, với tư cách là nhà phát triển và người quản lý codebase, là khiến cho không ai có thể làm hỏng production
Merge vào main có nghĩa là đã có người review, xem xét thay đổi, thảo luận thiết kế, sửa đi sửa lại nhiều lần rồi mới phê duyệt
Không ai xây tháp Eiffel một mình cả, và nếu ở nơi làm việc cứ đổ lỗi cho cá nhân thì chỉ sinh ra oán giận và văn hóa độc hại
Nếu đó là một kiểu hành vi lặp đi lặp lại thì đó là vấn đề để HR xử lý
Nếu trách nhiệm bằng 0 thì reviewer là vô dụng và chẳng có lý do gì phải review cả
Phân tán trách nhiệm đúng hơn là kết quả, còn mục đích cốt lõi là bắt lỗi — những lỗi mà một người làm một mình rất dễ bỏ sót nhưng từ hai người trở lên thì khó bị lọt hơn
Tuy vậy, tôi cho rằng trong phần mềm, review đang bị lạm dụng quá mức, và review không thể trở thành vật thay thế cho engineering
Tôi không nghĩ việc xem “phê duyệt mà không đọc code” là đồng nghĩa với “phê duyệt mà không suy nghĩ” là hoàn toàn chính xác
Ví dụ, nếu chương trình có kèm theo chứng minh tính đúng đắn, trong PR có thể đọc được các định nghĩa và định lý, CI dùng công cụ quyết định để xác minh chương trình và chứng minh khớp nhau, đồng thời cũng kiểm tra các yêu cầu phi chức năng như tỷ lệ tương phản, benchmark hiệu năng và độ trễ đuôi, còn thay đổi UI thì có thể dễ dàng tự tay thử trên prototype, thì sao
Ngay cả trong một thế giới như vậy, chưa chắc vẫn cần ám ảnh với việc đọc từng dòng code như hiện nay
Ngay cả bây giờ, đa số mọi người khi viết SQL cũng không kiểm tra tỉ mỉ xem query plan có khớp chính xác với điều mình muốn hay không
Bài gốc giống như đang cố “định nghĩa một thế giới lý tưởng nơi ta cảm thấy có thể triển khai mà không cần đọc code theo nghĩa đen”, rồi sau đó hỏi “làm sao để chuyển dần từ hiện tại sang thế giới đó một cách êm ái?”
Có thể tranh luận rằng cả ngành, hay một công ty hoặc codebase cụ thể nào đó, còn cách điểm đó bao xa, nhưng nếu nghiêm túc tưởng tượng về thế giới lý tưởng ấy thì hầu hết mọi người đều sẽ nghĩ ra được vài tiêu chí
Tôi hiểu vì xu hướng hiện tại của ngành nên người ta dễ hiểu thành “không suy nghĩ”, nhưng tôi không cho rằng đó là điều Charity thực sự muốn nói
Điều này thì dù test có tốt đến mấy cũng vẫn phải đọc code mới làm được
Nếu Alice đưa code vào, Bob review xong rồi Alice đi nghỉ, thì Bob phải hiểu đủ về phần đó và cảm thấy có trách nhiệm đủ để sửa hoặc thêm tính năng mới
Nếu Bob chỉ đóng dấu vào bản chứng minh tính đúng đắn thì sau đó anh ấy vẫn chưa sẵn sàng để làm việc với codebase đó
Dù có tham gia vào quá trình commit mà kiến thức hay cảm giác sở hữu không tăng lên thì thực chất cũng gần như chưa hề tham gia
Tuy nhiên compiler là công cụ có tính quyết định, còn LLM là công cụ không quyết định, có thể sinh ra những bài test đáng ngờ
Chúng ta vốn đã có những chương trình biến chỉ dẫn hoặc code do con người viết thành code hay biểu diễn trung gian mà máy có thể đọc được, và vì chúng bảo đảm mối quan hệ nào đó giữa đầu vào và kết quả sinh ra nên ta có thể tin đầu ra mà không nhất thiết phải kiểm tra thủ công
Có những lúc người ta đọc đầu ra của compiler bằng các công cụ như Godbolt để tối ưu hóa, nhưng ngoài ra thì phần lớn là không cần
Nếu thử làm điều này ở một tầng trừu tượng khác nhưng không quyết định, ban đầu có thể trông hợp lý, nhưng thực chất chỉ là bắt chước những bảo đảm về tính đúng đắn mà compiler cung cấp, và rồi sớm muộn gì cũng phát sinh vấn đề
Tranh luận về LLM, ở mức nền tảng hơn, là triệu chứng cho vấn đề rằng các ngôn ngữ lập trình quyết định hiện có chưa đủ khả năng biểu đạt và công cụ cũng chưa đủ hiệu quả
LLM có thể khiến người ta cảm thấy nó giải quyết được vấn đề đó, nhưng nó không phải lời giải thực sự; nó chỉ chồng thêm một lớp gián tiếp và chi phí hiệu năng lên trên một nền tảng vốn đã quá hạn chế
Tôi thấy nó giống một giả compiler đắt đỏ và nhiều lỗi, chạy trên một interpreter, mà bản thân interpreter đó lại chạy trên mã máy đã được biên dịch
Vì vậy tôi xem đây là một ngõ cụt về mặt kỹ thuật
Với doanh nghiệp, đó có thể là con đường tạo lợi nhuận ngắn hạn, nhưng tôi không tin nó sẽ cải thiện phần mềm hay mối quan hệ giữa con người với việc sử dụng và tạo ra phần mềm
Phần mềm không chỉ là công nghệ để tung ra sản phẩm
Sẽ hữu ích hơn nếu tách ra để nghĩ theo từng mục đích sử dụng
Nếu chỉ là đẩy thêm JavaScript mới cho UI của ứng dụng nội bộ thì tôi nghĩ không nhất thiết phải soi cả CSS