Code reviews - how to make it awesome part 2

Đây là phần hai tiếp nối phần một của bài viết tại đây

Nâng cao chất lượng của PR một cách chậm rãi

Các thành viên trong team luôn luôn muốn tìm cách nâng cao code của họ tuy nhiên, sự kiên nhẫn thì không phải ai cũng có được. Nếu chúng ta cứ chăm chăm vào bắt lỗi tất cả các vấn đề nhỏ nhặt hoặc muốn mọi thứ thật hoàn hảo. Nếu chúng ta chấm điểm PR theo thang A, B, C, D như đi học thì khi chúng ta nhận được một PR và chấm điểm PR đó là D. Chúng ta nên giúp author nên cao PR đó lên C hoặc B, mức đủ tốt để author có thể làm tiếp. Về lý thuyết chúng ta có thể đưa nó lên mức A nhưng nếu việc đó quá tốn effort để thực hiện(có thể phải viết lại phần lớn code) thì chúng ta có thể cân nhắc chỉ nâng chất lượng lên ở mức chấp nhận được. Bạn có thể nghĩ cách này sẽ tạo tiền lệ xấu hoặc nó sẽ giảm chất luượng dự án hay không? Điều này hoàn toàn không làm ảnh hưởng lớn lắm đến chất lượng. Thời gian đầu bạn có thể chấp nhận mức D và nâng nó lên C hoặc B nhưng càng về sau, mức khởi đầu của PR sẽ là C hoặc B. Chỉ một vài thời gian sau, teammate của bạn sẽ bắt kịp với chất lượng chung của dự án.

Giới hạn feedback về một vấn đề bị lập lại

Nếu như author gặp một lỗi do thói quen code, chúng ta thay vì comment lặp đi lặp lại một note trong PR vài chục lần thì có thể giới hạn lại 2, 3 feedback và nhắc author tự động thay đổi các chỗ khác thay vì tốn thời gian tìm tất cả các lỗi và copy,paste feedback.

Tôn trọng phạm vi của việc review

Chúng ta có thể bắt gặp các reviewer yêu cầu author sửa các phần code gần các line change, mặc dù họ không thay đổi gì đoạn code đó. Khi author sửa xong các comment thì chúng ta có thể thấy code tốt hơn một chút nhưng bây giờ PR đã vượt qua khỏi phạm vi thay đổi ban đầu, commit cũng thay đổi nội dung. Nếu việc này lập đi lập lại thường xuyên thì rất khó để tracking các sự thay đổi.

If a hungry little mouse shows up on your doorstep, you might want to give him a cookie. And if you give him a cookie, he’ll ask for a glass of milk. He’ll want to look in a mirror to make sure he doesn’t have a milk mustache, and then he’ll ask for a pair of scissors to give himself a trim…

-Laura Joffe Numeroff, If You Give a Mouse a Cookie Luật của quy tắc này khá đơn giản. Nếu PR không thay đổi line đó thì nó không thuộc phạm vi review. Ví dụ:

Dù bạn có thể nhận ra được sự xấu xí trong đoạn code đó và có thể author của PR cũng là người viết dòng code đó, nhưng nó vẫn ngoài phạm vi của sự review. Ngoại lệ duy nhất nếu như PR thực sự thay đổi bản chất của đoạn code cũ (thay đổi mục đích của hàm).

Luôn tìm cách để chia nhỏ các request change quá lớn.

Nếu như bạn nhận được một PR quá lớn với hàng loạt các thay đổi về cấu trúc, business của dự án thì nên yêu cầu chia nhỏ PR đó ra.

Đôi khi việc chia nhỏ PR khá là khó khăn vì với các task lớn, các function yêu cầu số lượng file change khá nhiều. Kinh nghiệm lúc này chúng ta nên nhóm các file lại, chia nhỏ theo cụm chức năng hoặc các tầng. Lập lại việc này khi mỗi PR được merge. Việc review các PR tầm 300- 400 lines change sẽ ít vất cả hơn các PR trên 800 lines change.

Hãy luôn khen ngợi nếu có cơ hội

Hầu hết khi review, chúng ta chỉ chăm chăm vào việc tìm lỗi sai mà quên mất đi đây là cơ hội để học hỏi và tán thưởng đồng nghiệp của họ nếu họ có những đoạn code chất. Hay nếu như author có tiến hộ rõ rệt sau từng PR hãy khen ngợi họ. Đây là động lực vô cùng tốt đối với fresher, ngoài ra còn giúp tăng sự gắng bó của team. Hãy xem những gợi ý sau:

• “I wasn’t aware of this API. That’s really useful!”
• “This is an elegant solution. I never would have thought of that.”
• “Breaking up this function was a great idea. It’s so much simpler now.”

Hãy approve nếu các vấn đề còn tồn tại không đáng kể

Một vài reviewer sẽ chỉ approve khi tất cả các lỗi, comment được fix nhưng điều này đôi khi quá cứng nhắc, nó có thể gây ra một sự lãng phí về thời gian và công sức của cả hai bên author và reviewer. Bạn có thể approve nếu các vấn đề sau đc thỏa mãn:

• Bạn không còn feedback nào.
• Còn một vài lỗi khá nhỏ (sai tên, sai chính tả).
• Các feedback là một gợi ý chứ không bắt buộc thay đổi.

Xử lý các cuộc tranh luận một cách chủ động

Điều tồi tệ nhất của code review là dẫn đến một cuộc tranh luận gay gắt. Reviewer không approve cho PR nếu các feedback không được sửa, author không đồng ý với các feedback và từ chối thay đổi source code. Nếu bạn gặp một trong các dấu hiệu sau thì bạn đang dính vào một cuộc review không được suôn sẻ lắm:

• Giọng điệu của hai bên bắt đầu gay gắt.
• Các feedback không giảm hoặc giảm ít sau mỗi round review.
• Nhận được reply trên hầu hết các feedback của mình.

Trong review code, chúng ta có rất nhiều lựa chọn: tập trung vào phần nào, đưa ra feedback như thế nào, khi nào có thể approve. Điều quan trọng ở đây là bạn nhận ra được ở mỗi bước, chúng ta đều có quyền lựa chọn, hoặc cứng rắn hoặc mềm dẻo để giải quyết vấn đề. Không một lý thuyết nào là chuẩn cho việc review tốt. Tùy từng hoàn cảnh mà chúng ta phải đưa ra được chiến lược review phù hợp. Nó phụ thuộc vào author, mối quan hệ giữa reviewer và author, văn hóa chung của team, chuẩn chung của team. Khi bạn dính vào một cuộc review căng thẳng. Hãy nhìn nhận lại quá trình xem lý do dẫn đến nó. Chú ý đến chất lượng các feedback của mình, hãy suy nghĩ xem quy trình đưa ra có cản trở bạn không và đi giải quyết nó. Chúc bạn may mắn và review ngày càng tốt hơn, được đồng nghiệp yêu mến nhiều hơn. LGTM, Merged.