Code review ที่ไม่ทำให้ใครเจ็บ

คนส่วนใหญ่เกลียด code review ไม่ใช่เพราะขี้เกียจหรือกลัวโดนจับผิด แต่เพราะมันมักกลายเป็นเวทีตัดสินมากกว่าเวทีแลกเปลี่ยนความรู้ เราจึงเห็น PR ที่ค้างอยู่สามวัน หรือ approve เปล่า ๆ เพราะไม่อยากมีเรื่อง

จริง ๆ แล้วการรีวิวไม่ได้มีไว้เพื่อตัดสินว่าใครเก่งกว่ากัน มันมีไว้เพื่อให้โค้ดก้อนนี้มีคนอ่านมากกว่าหนึ่งคนก่อนมันจะอยู่กับเราไปอีกหลายปี นั่นคือเป้าหมายทั้งหมด

สิ่งที่ควรดูก่อนเสมอ: เจตนาของการเปลี่ยนแปลง

ก่อนจะไล่ดูทีละบรรทัด ให้ถามตัวเองก่อนว่า PR นี้พยายามแก้ปัญหาอะไร แล้ววิธีที่เขาเลือกมันเหมาะสมกับปัญหานั้นไหม คอมเมนต์เรื่องช่องไฟหรือการจัดเรียงไฟล์นั้นถูกเสมอ แต่มันเป็นเรื่องรอง

ความถูกต้อง — โค้ดนี้ทำสิ่งที่ตั้งใจจะทำจริงไหม มี edge case ไหนที่หลุด
ความปลอดภัย — มีช่องโหว่สิทธิ์ รับ input โดยไม่ validate หรือ log ข้อมูลส่วนตัวออกมาไหม
ความเข้าใจง่าย — อีกหกเดือนข้างหน้าคนที่ไม่เคยเขียนจะอ่านรู้เรื่องไหม
ความสอดคล้อง — เข้ากับแพตเทิร์นที่โปรเจกต์ใช้อยู่หรือสร้างทางที่สามขึ้นมา
รสนิยมส่วนตัว — อยู่ล่างสุดเสมอ และควรให้ linter จัดการแทนมนุษย์

เขียนคอมเมนต์อย่างไรให้ไม่เหมือนการตัดสิน

คอมเมนต์ที่เขียนว่า "ตรงนี้ผิด" กับคอมเมนต์ที่เขียนว่า "ตรงนี้ถ้า list ว่าง จะเกิดอะไรขึ้นครับ" ให้ผลต่างกันมาก อันแรกคือคำตัดสิน อันหลังคือคำถามที่เปิดทางให้อีกฝ่ายคิดเอง

แบบที่ทำให้เกร็ง	แบบที่ชวนคุย
ทำไมไม่ใช้ map	ตรงนี้ใช้ map แทนจะสั้นลงไหม หรือมีเหตุผลที่ต้องใช้ for
โค้ดนี้อ่านยาก	ส่วนนี้อ่านหลายรอบกว่าจะเข้าใจ แยกออกเป็นฟังก์ชันจะดีขึ้นไหม
นี่มันผิด	ตรงนี้ถ้า user เป็น null น่าจะ throw ใช่ไหมครับ
ทำไมไม่ทำแบบที่ผมบอก	ผมสงสัยเรื่อง transaction ตรงนี้ พอจะเล่าให้ฟังได้ไหม

แยกความเห็นออกเป็นระดับ

ปัญหาใหญ่ของการรีวิวคือคนเขียนแยกไม่ออกว่าคอมเมนต์ไหนคือต้องแก้ กับอันไหนคือแค่ความชอบส่วนตัว วิธีแก้ง่ายมากคือติดป้ายกำกับไว้ตรง ๆ

[blocking] — ต้องแก้ก่อน merge
[suggestion] — คิดว่าดีขึ้น แต่ตัดสินใจเองได้
[question] — อยากเข้าใจ ไม่ได้จะบอกว่าผิด
[nit] — จุกจิกมาก จะแก้หรือไม่ก็ได้
[praise] — ตรงนี้เขียนดีมาก อย่าลืมเขียนอันนี้

แค่คำนำห้าคำนี้ก็ลดความกดดันได้เยอะมาก เพราะเจ้าของ PR จะรู้ทันทีว่าอันไหนต้องทำ อันไหนคุยกันเฉย ๆ

คนเปิด PR ก็มีหน้าที่

รีวิวที่แย่ครึ่งหนึ่งเกิดจาก PR ที่รีวิวยาก ถ้าเปิด PR ที่มี 60 ไฟล์เปลี่ยนและไม่มีคำอธิบาย ก็ไม่ต้องแปลกใจที่จะได้ approve มาแบบขอไปที

ก่อนกด Create pull request

○แยกการจัดรูปแบบออกจากการเปลี่ยน logic เป็นคนละ PR
○เขียนคำอธิบายว่าแก้อะไร ทำไมแก้แบบนี้
○ชี้จุดที่อยากให้รีวิวเอาใจใส่เป็นพิเศษ
○รันเทสต์และ linter ผ่านแล้ว
○อ่าน diff ของตัวเองหนึ่งรอบก่อนส่งให้คนอื่น

เวลาเห็นไม่ตรงกัน ให้กำหนดเส้นตาย

กฎที่ใช้ได้ผลคือ ถ้าคุยกันไปเกินสองรอบแล้วยังไม่จบ ให้ย้ายไปคุยกันต่อหน้าบน call สิบนาทีต่อหน้าจอมักจบได้เร็วกว่าคอมเมนต์สิบรอบ และถ้ายังตกลงกันไม่ได้จริง ๆ ให้เชิญคนที่สามมาตัดสิน ดีกว่าปล่อยให้ PR ค้างอีกสามวัน

เป้าหมายของ code review ไม่ใช่โค้ดที่สมบูรณ์แบบ แต่คือโค้ดที่ดีกว่าตอนที่เปิด PR มา อย่างมีนัยสำคัญ

ถ้าทุกคนในทีมจำไว้แค่นี้ การรีวิวจะเปลี่ยนจากด่านที่น่ากลัว กลายเป็นช่วงเวลาที่ทุกคนได้เรียนระบบเพิ่มขึ้นอีกนิดหน่อย

อยู่ในซีรีส์: ช่างฝีมือโค้ด

อ่านตอนต่อไป

ความคิดเห็น

ต้องมีบัญชีก่อนจึงจะแสดงความคิดเห็นได้ เพื่อให้เจ้าของบทความรู้ว่ากำลังคุยกับใครและให้ระบบกันสแปมทำงานได้

เข้าสู่ระบบหรือสมัคร