เวลามี Merge Request / Pull Request มีแนวทางการตรวจยังไง

พอดีเห็น Post ในกลุ่มสมาคมโปรแกรมเมอร์ไทย เลยคิดว่าเอา Comment ตัวเองในนั้นมองลอง Rewrite เขียน Blog ใหม่อีกทีน่าจะดีครับ ^__^

Prerequisite

  • Code ควรวางโครงสร้างมาดี ขอผลแยก layer dto / repo / service / controller จะได้หาง่าย แต่ถ้าไม่ดีควรต้องจัดการหาเวลา Refactor / Restructure / Rearcitect เพื่อลด Technical Debt ครับ
  • เรามีความเข้าใจในภาษานั้นๆ ที่จะตรวจระดับนึง dotnet นะ ถ้าผมไปทำ spring / nuxt อะไรพวกนี้ ไม่กล้า review เอง skill ไม่ถึง 5555

การ Review นอกจากช่วยทำให้ Code ดีขึ้นแล้ว ยังเป็นการแชร์ความรู้ด้วย
แต่เราต้องมีพื้นฐาน หรือ Fundamential ที่ดีก่อนครับ

แนวทางการตรวจ Merge Request / Pull Request

  • ผมจะเน้น 1 พวกที่แก้ Schema เอาไว้ตรวจก่อนเลยว่า Script ครบไหม Entity / DTO และอะไรที่ทำซ้ำซ้อน เคยเจอ เรื่องเดียวกัน SA-DEV ไปทำ Table 3 ชุด มาทะเลาะกันตอน Merge จริงมันเป็นปัญหา Product ควรคุมภาพรวม ควรจะเห็นก่อนมา merge
  • ดูเทสก่อน ว่าที่เค้าเพิ่มมาเขียนไหม ถ้าเขียน ผมเชื่อคนที่ส่งนะ เพราะเราไม่รู้ Req Logic เค้า แต่จะไปดู Test เดิมที่เค้าแก้ แล้วถามกลับ ว่าคุยกับอีกคนแล้วยัง ที่เจอมันพังๆ Req ใหม่มทำ ทำ Test พอ Test เดิมพัง แก้ผลลัพธ์ฺ ไม่ได้ตกลงอะไรกับคนเก่า หรือ ทำความเข้าใจ และส่ง
  • SQL ถ้ามี Custom ต้องไปดูว่าทำ Platform Specific ไหม ที่เจอเมื่อเดือนก่อนทำ Custom Query ที่ใช้ Syntax ของ MSSQL พวก DB2 PostgreSQL ตุยตอน Runtime
  • จากนั้นเข้าไปได้ดู Service / Controller หลักๆจะอะไรที่มาเป็นก้อนๆ แบบพวก For จับ DB / API คับ หรือเรียกใช้ API Lib ไม่เหมาะสม และที่สำคัญต้องจับการ Copy Code ผมเจอหลายคนไม่ร้จักการ override (พวก oop) copy แปะมาเลย ตัวการใหญ่ที่ทำบวมเลย และลำบากคน MA ด้วย เพราะถ้า copy ไป 20 จุด คนตามแก้ เค้าอาจจะ 5 / 10 ถ้าไม่คุมยาวครับ

ของ dotnet ผมใช้ตัวนี้ https://github.com/thangchung/clean-code-dotnet มา Ref และจดเพิ่มๆเอาลงใน Wiki กลางครับ

กรณีที่ MR / PR นั้นไฟล์มันมาแบบเยอะมาก ทำยังไง ?

จากที่ผมเคยเจอหลัก 100 - 3000 ที่ผมเคยเจอมากสุดนะ ถ้าเริ่มต้องทำยังไง

  • สูดหายใจลึกๆ
  • ยิ่งไฟล์เยอะ หมายถึงว่า Review ทดสอบจะนานขึ้นด้วย ถ้ามีการทำ Test ถือว่าโชคดีมาก ขอผมเคยดึงไปมากสุด 2-3 Week
  • ถ้าโดนใครเร่ง ให้คนเร่งเมล์บอกขอ merge และต้องรับประกันด้วย ถ้ามันพังจะมาช่วย Support เต็มๆที่ เคยหยวนๆเอาเข้าไป ปรากฏมันพัง แล้วทั้ง pm / sale / cs มารุมด่าผมด่าแรง (คนส่งเค้า Senior กว่ารอด) แล้วไม่กลับมาขอโทษ ว่าปล่อยได้ไง เพราะ 20-30 Site ใช้ Code Base เดียวกัน
  • อีกเคสที่ต้องระวัง ถ้าปล่อยเข้า Branch หลักแล้ว ทีมงานเดิมจะมองว่าไม่ใช่ความรับผิดชอบของเค้า อันนี้ปัญหาเรื่องคน การเมืองในองค์กรแล้ว

สุดท้ายแล้ว การที่ Merge มันมาเยอะๆ ขนาดนั้นต้องมองกลับไปที่การวางแผนแล้ว ว่าทำไมปล่อย ให้ Branch มาใหญ่ขนาดนั้นแล้ว Merge

อ๋อ แล้ว Code review ไม่ใช่การมานั่งจับผิดโค้ด แต่เป็นกระบวนการแบ่งปันองค์ความรู้ให้กันและกัน

Key สำคัญของการทำ Code Review พยายามมองหาจุดที่สามารถ Improve Code หรือ ตรวจสอบ Impact ที่เกิดขึ้นได้ ทั้งในส่วน Technical หรือ ในส่วน Requirement เองเป็นต้น รวมถึงการแชร์ความรู้ทั้ง Pattern / จุดที่ควรระวัง หรือ จุดที่ควรเติม Test เพิ่ม เพื่อลดความเสี่ยงของการเกิดปัญหาในอนาคต แต่ที่สำคัญเลย รีวิวโดยไม่ใช้อารมณ์ หรือ อคติใส่ลงไป ทั้งผู้ Review และผู้ที่ถูก Review ด้วย

นอกจากการเอ๊ะ ตั้งคำถามจากการ Review ไม่ว่าจากทั้ง ผู้ Review และผู้ที่ถูก Review จะช่วยให้ได้ทั้งความรู้ใหม่ บางทีเรารีวิวไป แต่อ๋อมันมี Pattern นี้ด้วย เป็นการแชร์องค์ความรู้ และเป็นการพัฒนาตัวเอง ซึ่งสุดท้ายมันส่งผลดีกับ Codebase และ Product ด้วยครับ

Reference

Discover more from naiwaen@DebuggingSoft

Subscribe to get the latest posts sent to your email.

Tags

Related Posts