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 มา อย่างมีนัยสำคัญ
ถ้าทุกคนในทีมจำไว้แค่นี้ การรีวิวจะเปลี่ยนจากด่านที่น่ากลัว กลายเป็นช่วงเวลาที่ทุกคนได้เรียนระบบเพิ่มขึ้นอีกนิดหน่อย
ความคิดเห็น
กำลังโหลดความคิดเห็น