- Code review nghe có vẻ là một ý tưởng hay, đúng không?
- Nhờ code review, hai lập trình viên có thể cùng xem mã, phát hiện vấn đề và có cơ hội chia sẻ hiểu biết về quá trình phát triển của dự án
- Reviewer có thể học được các mẹo hữu ích khi xem kỹ mã của tác giả, hoặc tìm thấy cơ hội để chỉ lại cho tác giả những mẹo hữu ích
- Nhưng đó là cách mà các reviewer code review ở “phe ánh sáng” sử dụng. Họ dùng code review để cải thiện mã và nâng cao kỹ năng tập thể của các lập trình viên
- Code review cũng có thể là một công cụ mạnh mẽ phục vụ mục đích hoàn toàn khác. Nếu reviewer chuyển sang “phe bóng tối”, họ có thể dùng nhiều cách để cản trở hoặc trì hoãn việc cải thiện mã
- Họ cũng có thể theo đuổi những mục đích cá nhân khác, như hành hạ tác giả bản vá hoặc khiến họ hoàn toàn nản chí
- Nếu bạn là reviewer mới chuyển sang “phe bóng tối”, có thể bạn vẫn chưa nghĩ đến mọi khả năng
- Vì vậy, bài viết này đưa ra một danh sách các phản mẫu dành cho reviewer code review phe bóng tối
[Phản mẫu]
The Death of a Thousand Round Trips
- Trong lúc đọc mã, cứ hễ phát hiện một chi tiết nhỏ là lập tức để lại bình luận review rồi dừng đọc ngay
- Lập trình viên cần mẫn sửa chi tiết nhỏ đó và gửi lại bản vá đã chỉnh sửa
- Reviewer bắt đầu đọc phiên bản mới, rồi lại phát hiện một chi tiết nhỏ khác. Đó là thứ lẽ ra có thể nói ngay ở vòng review đầu tiên, nhưng đã không nói. Reviewer chỉ ra chi tiết đó rồi lại dừng đọc tiếp
- Cứ lặp lại như vậy cho đến khi lập trình viên mất hết hy vọng
The Ransom Note
- Có vẻ bản vá này đặc biệt quan trọng với lập trình viên
- Nhưng với reviewer thì nó không quan trọng đến thế. Vì vậy reviewer đang ở vị thế có quyền lực
- Reviewer có thể giữ thay đổi mà lập trình viên muốn như một con tin cho đến khi các việc bổ sung mà reviewer coi trọng được hoàn thành
- Những việc bổ sung đó thực ra không nhất thiết phải nằm trong cùng commit, nhưng lại là việc reviewer thấy quan trọng
The Double Team
- Một bản vá, hai reviewer
- Mỗi khi một reviewer yêu cầu thay đổi, lập trình viên đều ngoan ngoãn sửa. Rồi đến lượt reviewer còn lại phàn nàn
- Hai reviewer thay phiên nhau đưa ra các yêu cầu mâu thuẫn với nhau
- Nhưng họ luôn bình luận hướng về phía lập trình viên. Họ tránh tranh luận trực tiếp với nhau trong luồng review
- Mục tiêu là xem họ có thể đẩy qua đẩy lại lập trình viên bao nhiêu lần trước khi người đó bỏ cuộc
The Guessing Game
- Có một vấn đề và có nhiều cách khác nhau để giải quyết nó
- Lập trình viên chọn một cách giải quyết rồi gửi bản vá
- Reviewer chỉ trích chính giải pháp đó vì những lý do không liên quan đến việc nó có giải quyết được vấn đề hay không
- Ví dụ như vi phạm một nguyên tắc thiết kế mơ hồ, hoặc không tương thích với các kế hoạch phát triển trong tương lai
- Nếu lời chỉ trích đủ mơ hồ, lập trình viên sẽ không biết phải sửa bản vá hiện tại thế nào để giải quyết được các chỉ trích đó
- Khi ấy, cách tốt nhất từ phía lập trình viên là chọn một giải pháp hoàn toàn khác để xử lý vấn đề gốc rồi triển khai lại
- Và rồi reviewer lại tiếp tục bác bỏ nó theo cùng một cách vô ích như trước
The Priority Inversion
- Ở vòng code review đầu tiên, chỉ ra những thứ nhỏ nhặt và đơn giản trước
- Chờ đến khi lập trình viên sửa xong chúng rồi mới nêu ra các vấn đề nghiêm trọng
- Có một vấn đề nền tảng khiến phần lớn bản vá phải được viết lại hoàn toàn. Điều đó cũng đồng nghĩa nhiều chỉnh sửa nhỏ đã làm trước đó sẽ phải bỏ đi
- Chẳng có thông điệp nào thể hiện rõ hơn câu “công sức của bạn không được mong muốn, và thời gian của bạn không quý giá” bằng việc bắt ai đó làm rất nhiều rồi lại vứt bỏ nó
- Chỉ riêng điều này cũng có thể đủ khiến lập trình viên bỏ cuộc
The Late-Breaking Design Review
- Một công việc cực kỳ phức tạp đã được triển khai suốt nhiều tuần hoặc nhiều tháng qua dưới dạng nhiều bản vá riêng lẻ
- Reviewer không đồng ý với thiết kế của toàn bộ công việc đó, nhưng hoặc là đã không tham gia thảo luận ban đầu, hoặc là ở phe thua trong cuộc thảo luận
- Nhưng giờ đây reviewer được yêu cầu xem một phần nhỏ nhưng quan trọng ở giữa công việc đó, chẳng hạn một bản sửa lỗi đơn giản cho bug đang chặn rất nhiều lập trình viên. Với reviewer, đây là cơ hội
- Reviewer yêu cầu phải biện minh cho thiết kế của toàn bộ công việc đã làm đến nay
- Nếu lập trình viên chỉ phụ trách một phần của toàn bộ công việc nên không biết câu trả lời, đó không phải vấn đề của reviewer. Reviewer sẽ không bấm nút “phê duyệt” cho đến khi thấy thỏa mãn
The Catch-22
- Nếu là một bản vá lớn thì quá khó đọc. Hãy yêu cầu chia nó thành các phần nhỏ self-contained
- Nhưng nếu có nhiều bản vá nhỏ thì một số trong đó lại không có ý nghĩa nếu đứng riêng. Hãy yêu cầu gộp lại
- Nếu có vẻ như tồn tại một kiểu đánh đổi nào đó liên quan đến trường hợp cụ thể, bạn có thể tận dụng nó để tìm lý do phàn nàn
- Ví dụ, nếu mã được viết cho dễ nhận biết thì hiệu năng sẽ kém; còn nếu được tối ưu tốt thì lại khó đọc và khó bảo trì
The Flip Flop
- Có một kiểu thay đổi mà mọi người thường xuyên thực hiện trên cùng một phần mã
- Reviewer trước đây đã từng review kiểu thay đổi này nhiều lần
- Một thay đổi tương tự khác xuất hiện. Tác giả đã xem rất kỹ các thay đổi trước đó, cẩn thận làm theo mẫu có sẵn, và chọn reviewer có vẻ quen thuộc với khu vực này
- Reviewer bỗng nhiên phản đối một khía cạnh của thay đổi mà trước đây chưa từng coi là vấn đề. Chỉ làm theo mẫu cũ giờ không còn đủ nữa
- Nếu tác giả chỉ ra sự thiếu nhất quán của reviewer bằng cách đưa ra các thay đổi giống hệt trong quá khứ thì sao?
- Reviewer sẽ nói: “Đúng vậy. Cái đó cũng nên được thay đổi”
- Reviewer cần cẩn thận để không tự nguyện đứng ra sửa tất cả các trường hợp hiện có. Nếu may mắn, lập trình viên sẽ hiểu đó là chỉ thị để tự mình sửa các trường hợp cũ, giúp reviewer tiết kiệm rất nhiều công sức
Nhưng nói nghiêm túc thì ...
- Đến đây, bài viết này chỉ là một trò đùa. Nó cũng không nhằm gợi ý rằng reviewer cố ý làm những điều tệ hại này
- Phần lớn các mô tả là sự phóng đại từ những gì reviewer thực sự làm, hoặc là sự phóng đại cảm giác của người gửi bản vá khi bị dồn nén và thất vọng
- Ý thật sự là: đừng làm như vậy!
- Hãy cố giảm thiểu số vòng qua lại, yêu cầu viết lại phần quan trọng trước khi soi mói những chi tiết nhỏ nếu cần, và khi phê bình bản vá thì nên đưa ra đề xuất mang tính xây dựng về phiên bản nào bạn có thể chấp nhận
- Nếu bạn có ý kiến về toàn bộ codebase, hãy thảo luận tổng thể với tất cả các lập trình viên thay vì vin vào bản vá của một người để bắt bẻ
- Nếu lỡ làm như vậy, hãy tự nhận ra điều đó. Hãy thừa nhận rằng mình đã vô tình khiến cuộc sống của lập trình viên khó khăn hơn, xin lỗi, và cố gắng trở nên hữu ích hơn
- Vấn đề cốt lõi là lạm dụng quyền lực
- Khi một lập trình viên trở thành reviewer cho bản vá của người khác, họ có được một quyền lực tạm thời. Reviewer có quyền ngăn bản vá đó được commit
- Quyền lực đi kèm trách nhiệm. Chỉ nên dùng quyền lực cho đúng mục đích và trong phạm vi cần thiết
- Hầu hết các phản mẫu này (hoặc các hành vi nhẹ hơn mà bài viết châm biếm) đều là lạm dụng quyền lực. Reviewer dùng quyền lực tạm thời đối với lập trình viên như đòn bẩy để theo đuổi một chương trình cá nhân không liên quan hoặc đi ngược với việc cải thiện bản vá
- Chương trình cá nhân khác nhau tùy từng phản mẫu: có thể là công việc không liên quan, sở thích cá nhân về phong cách, sự lười biếng, sự chống đối thay đổi, hoặc ác cảm cá nhân với người gửi bản vá
- Nếu người gửi bản vá trước đây từng là reviewer và đã có những hành vi xấu như vậy thì sự ác cảm đó thậm chí có thể là có cơ sở. Vì thế reviewer phải dùng quyền lực của mình một cách thận trọng
- Nếu các lập trình viên rơi vào vòng xoáy trả thù lẫn nhau thì dự án phần mềm sẽ đi đến diệt vong
Gatekeeping code review
- Đến đây, bài viết chủ yếu tập trung vào code review giữa các đồng nghiệp
- Reviewer và người gửi bản vá là đồng cấp, không chịu trách nhiệm lẫn nhau và cũng không có quyền quyết định cuối cùng với codebase
- Vì vậy quyền lực có được từ code review chỉ là tạm thời
- Trong bối cảnh peer review, reviewer và tác giả về cơ bản nên có cùng mục tiêu
- Nếu có bất đồng nghiêm trọng về việc nên thêm tính năng nào, nên mài giũa đến mức nào trước khi duyệt, hay phong cách code nào là chấp nhận được, thì cần xử lý chúng ngoài bối cảnh code review
- Nhưng trong những kiểu code review khác thì không như vậy. Đặc biệt là khi người ngoài dự án gửi những bản vá không được yêu cầu
- Kịch bản này thường xảy ra trong phần mềm tự do
- Vì bất kỳ ai trên thế giới cũng có thể sửa mã nguồn và một số người sẽ cố gắng gửi lại các thay đổi của họ
- Nhưng điều này cũng có thể xảy ra trong các bối cảnh khác
- Trong một công ty phát triển mã nguồn sở hữu độc quyền (proprietary), lập trình viên của đội này có thể gửi bản vá cho dự án của đội khác và hy vọng gặp may
- Trong những tình huống như vậy, khả năng người nhận bản vá vốn dĩ không muốn thay đổi đó hoặc hoàn toàn không đồng ý với cách thực hiện, nên sẽ không chấp nhận bản vá, cao hơn rất nhiều
- Khi đó, đây không phải là lạm dụng quyền lực tạm thời được trao cho một reviewer đồng cấp, mà là việc thực thi chính đáng quyền lực lâu dài với tư cách người chịu trách nhiệm dự án
- Hướng đi của dự án phần mềm của tôi là do tôi quyết định
- Khi reviewer đóng vai trò “gatekeeping” như vậy, không phải lúc nào cũng sai nếu họ phê bình bản vá vì nó vi phạm các nguyên tắc thiết kế chung hoặc yêu cầu hiện có
- Có thể họ thực sự không biết cách giải quyết vấn đề đó theo cách phù hợp với các yêu cầu
- Thực tế, đó có thể chính là phần khó, và cũng là lý do duy nhất đến giờ tôi vẫn chưa tự làm thay đổi tương tự
- Tuy nhiên, ngay cả trong bối cảnh gatekeeping, việc áp dụng “The Guessing Game” mà không giải thích vẫn là bất lịch sự
- Nhìn chung, tôi cố gắng giải thích rằng lập trình viên đã bỏ sót phần khó, và nếu bản thân tôi cũng không biết câu trả lời thì tôi sẽ nói thẳng điều đó
- Dĩ nhiên không có lời bào chữa nào cho kiểu cản trở thụ động-aggressive như “The Death of a Thousand Round Trips”
- Nếu thực sự không muốn đưa bản vá vào mã nguồn chút nào, và bạn đang ở vai trò gatekeeping có thẩm quyền chính đáng để đưa ra quyết định đó, thì bạn nên nói rõ bằng lời để người gửi không tiếp tục lãng phí thời gian
Disclaimer
- Trong nhiều năm, tôi đã gom góp ghi chú cho bài viết này từ các code review mà tôi từng tham gia (ở cả hai phía), các code review tôi quan sát giữa người khác, và cả những code review mà tôi chỉ nghe kể trong các cuộc trò chuyện
- Vì vậy đây không nhắm vào bất kỳ cá nhân cụ thể nào đã review mã của tôi gần đây
13 bình luận
Có lẽ bất ngờ là điều này không hẳn là cường điệu....
Cái này tôi thật sự đã từng trải qua, suýt nữa thì bỏ nghề lập trình luôn, thật sự đã rất khó khăn để gượng dậy.
Đọc bài này xong suýt bị PTSD.
Có vẻ như bạn đã thu thập ghi chú cho bài viết này rất tốt trong suốt thời gian qua!!
Chỉ đọc thôi cũng đã ở mức ngược đãi tinh thần...
Ý là câu cuối mới là trọng tâm đúng không? haha.....
Wow... tôi tưởng mình sắp phát điên luôn ấy;;
Mấy chuyện kiểu này, nếu đến những nơi làm
si + smthì ngay cả ở Hàn Quốc cũng có thể thấy khá phổ biến. Thường người ta gọi đó là bắt nạt người mới. Những kẻ xấu tính để giữ chén cơm của mình mà bày đủ trò.Có nhiều cách thật ác độc.
Về lâu dài, những người làm mấy việc kiểu đó mà không có lý do hợp lý thì либо là 1) bị gạt khỏi mạng lưới quan hệ của giới lập trình viên từ sớm, hoặc 2) dù nhân cách tệ hại nhưng năng lực quá xuất sắc nên đang nắm phần việc lớn và khó bị loại ra, vì thế sẽ có ai đó đóng vai trò cầu nối, làm đầu mối giao tiếp để duy trì kết nối, nên trạng thái đó mới được giữ nguyên; nếu người trung gian ấy biến mất vì bất kỳ lý do gì thì cũng chẳng bao lâu sau sẽ nhanh chóng bị loại ra. Dù có giỏi giang đến đâu thì cuối cùng vẫn là con người tụ lại với nhau để làm gì đó thì tiền mới lưu chuyển, mà tiền lưu chuyển thì con người cũng phải qua lại với nhau, nên người không thể hòa hợp với người khác tất yếu sẽ bị loại khỏi tập thể và tụt lại phía sau.
Thường thì những người sống sót lâu trong một tập thể dù nhân cách tồi tệ hay lầm tưởng rằng mình tồn tại được là vì bản thân là một kiểu nhân vật ghê gớm nào đó, như Sherlock trong phim truyền hình, một “high-functioning sociopath”, nhưng thực ra chỉ là vì họ còn giá trị để lợi dụng nên những người xung quanh cố nhịn mà dùng mà thôi; khi giá trị sử dụng biến mất thì mối quan hệ sẽ trở thành kiểu “làm việc chung với cậu đúng là kinh khủng, đừng bao giờ gặp lại nữa”. Sherlock do Cumberbatch đóng cũng chỉ là một sociopath hấp dẫn dưới góc nhìn của người đứng ngoài mà thôi; nếu không có những người bên cạnh không từ bỏ và vẫn trân trọng Sherlock thì đơn giản là chẳng có câu chuyện nào để kể cả.
Những người có nhân cách tệ hại mà vẫn sống sót lâu thường hay ngộ nhận rằng mình tồn tại được là vì bản thân là một thứ gì đó ghê gớm, kiểu một sociopath chức năng cao như Sherlock trong phim; nhưng thực ra chỉ là vì còn giá trị để lợi dụng nên xung quanh mới cắn răng chịu đựng và dùng đến, còn khi giá trị lợi dụng biến mất thì mối quan hệ sẽ thành kiểu "ở cùng nhau thật bẩn thỉu, đừng bao giờ gặp lại nữa". ==> Câu này quá đắt. Phải nhớ mới được.
Điều này làm tôi nhớ đến hành vi bắt nạt nơi công sở, hay nói cách khác là power harassment.
Bảo là hài hước nhưng đọc bài mà bực lên thấy rõ..