Review checklist

นี่คือ checklist ที่ไล่ดูด้วยตา บน diff ไหนก็ได้ ของคุณเองหรือของ AI หลังจากเครื่องมือใน Verification Tools รันผ่านและสะอาดแล้ว หน้านี้ตั้งใจให้พอดี 1 หน้าเวลาพิมพ์ — checklist ที่ไม่มีใครเปิดดูจริงตอน review ก็ไร้ค่า ทุกบรรทัด link ไปบทที่อธิบายว่า ทำไม เพื่อให้หน้านี้ยังคงเป็นแค่ list ไม่ใช่การสอนซ้ำ

Errors

• ทุก error return ถูกเช็กก่อนใช้ค่าที่อยู่ข้างๆ มันหรือเปล่า?
• มี error ไหนถูกจัดการสองรอบ — ทั้ง log และ return ขึ้นไปด้วยหรือเปล่า? ดู Error Values
• มี := ใน if/for ที่บังเอิญ shadow err ด้านนอกไหม? ดู Variables & Types
• concrete error type ถูก assign ให้ตัวแปรชนิด error แล้ว return ไปตรงๆ โดยไม่ตรวจสอบไหม? ดู A nil pointer inside an interface is not nil
• ทุก errors.As call ส่ง pointer (&target) ไม่ใช่ target เฉยๆ หรือเปล่า? ดู Wrapping Errors
• recover() ถูกเรียกตรงภายใน deferred function เอง — ไม่ใช่ผ่าน helper อีกชั้นหนึ่งใช่ไหม? ดู Panic and Recover
• มี package นอก main ตัวไหนเรียก log.Fatal หรือ os.Exit ทำให้ defer ของ caller ถูกข้ามไปไหม? ดู Panic and Recover

Concurrency

• ทุก goroutine มี exit condition ที่มองเห็นได้ไหม — channel ที่ปิดแล้ว, ctx ที่ cancel แล้ว, หรือค่าที่มันรออยู่? ดู Goroutines
• wg.Add ถูกเรียกก่อน goroutine เริ่ม จาก goroutine เดียวกับที่จะเรียก wg.Wait หรือเปล่า? ดู Goroutines
• ทุก goroutine ที่ panic ได้ recover จากภายในตัวมันเองหรือเปล่า? ดู Panic and Recover
• มี type ที่มี sync.Mutex (หรือคล้ายกัน) ตัวไหนถูกใช้หรือส่งต่อแบบ value ไหม? ดู The sync package
• ถ้า loop launch goroutine หรือ defer closure, go directive ใน go.mod รับประกัน per-iteration variable จริงหรือเปล่า (1.22+) หรือ loop variable ถูก capture ไว้อย่างชัดเจนแล้ว? ดู Functions
• break เปล่าๆ ใน select ใน for ไปถึง loop ที่มันดูเหมือนจะเล็งไปจริงหรือเปล่า? ดู Control Flow
• background work ใช้ context.Context ของ request ซ้ำทั้งที่ควรตัดด้วย context.WithoutCancel หรือถูกเก็บไว้ใน struct field หรือเปล่า? ดู Context

Resources

• ทุก defer ที่ close/unlock/release อะไรบางอย่าง ผูก scope กับการ acquire ครั้งเดียว ไม่ใช่สะสมข้าม loop ใช่ไหม? ดู io and bufio
• resource (resp.Body, ไฟล์, lock) ถูกใช้หลังจากเช็ก error ตอน acquire แล้วเท่านั้นใช่ไหม? ดู The client side
• ทุก http.Client ที่ใช้นอกเหนือจาก script เร็วๆ ตั้ง Timeout หรือเปล่า? ดู The client side

API contract

• resp.StatusCode ถูกเช็กก่อนเชื่อ response body ว่าเป็นข้อมูลจริงไหม? ดู The client side
• omitempty อยู่บน time.Time หรือ struct field ที่คาดหวังให้มัน omit zero value ไหม? ดู encoding/json
• ทุก field ที่ JSON contract ต้องการถูก export (และ tag) จริงไหม? ดู encoding/json
• nil slice ถูก return ในจุดที่ contract ต้องการ [] หรือกลับกันไหม? ดู encoding/json

Types

• append บน sub-slice เสี่ยงทับ backing array ของ parent slice ไหม? ดู Slices
• map ถูกเขียนก่อนที่จะ initialize ด้วย make หรือ literal หรือเปล่า? ดู Maps
• มี call site ไหนยังสมมติว่า maps.Keys/maps.Values คืน slice แทนที่จะเป็น iter.Seq ไหม? ดู Iterators

Tooling & style

• time.Sleep/duration literal ทุกตัวระบุ unit ชัดเจนไหม (5 * time.Second ไม่ใช่ 5 เปล่าๆ)? ดู time
• interface ใหม่มี implementation จริงมากกว่าหนึ่งตัวไหม — หรือ any/concrete type จะง่ายกว่า? interface{} ถูกใช้ในจุดที่ any อ่านง่ายกว่าไหม? receiver ตั้งชื่อตาม type ไม่ใช่ this/self ใช่ไหม? ดู Interfaces และ Methods

🟦 checklist ช่วยลดขอบเขตการค้นหา ไม่ได้แทนที่มัน

ทุกบรรทัดด้านบน map ไปยังหนึ่งใน 24 pattern ใน AI Bug Catalog — ครึ่งหนึ่งไม่มีเครื่องมือไหนในคอร์สนี้จับได้อัตโนมัติเลย รัน list นี้กับทุก diff ไม่ว่า linter จะรายงานสะอาดแค่ไหนก็ตาม

'ทุก http.Client set Timeout หรือเปล่า' จัดอยู่กลุ่มไหน และทำไมถึงเป็นกลุ่มนั้นแทนที่จะเป็น 'API contract'?

API contract — เพราะเป็นเรื่องว่า caller คาดหวังอะไรจาก response ได้บ้าง Resources — Client ที่ไม่มี Timeout อาจถือ connection (ซึ่งเป็น resource) ค้างไว้ไม่มีที่สิ้นสุด Types — http.Client เป็น struct type Tooling & style — เป็นเรื่อง code style ไม่ใช่ correctness

ทำไม checklist ถึงบอกให้เช็ก 'recover() ถูกเรียกตรงข้างในตัว deferred function เลยหรือเปล่า' แทนที่จะเช็กแค่ 'โค้ดเรียก recover() ที่ไหนสักแห่งหรือเปล่า'?

recover() จะหยุด panic ได้ก็ต่อเมื่อถูกเรียกตรงจาก deferred function เท่านั้น — ถ้ามี helper คั่นอีกชั้นเดียว มันจะไม่ทำงานแบบเงียบๆ recover() ถูก deprecate ไปแล้วใน Go เวอร์ชันปัจจุบัน recover() ต้องถูกเรียกก่อน defer ไม่ใช่หลัง ไม่มีความต่างกันเลย — สองแบบนี้เช็กสิ่งเดียวกัน

checklist บอกให้เช็ก go directive ใน go.mod สำหรับ bug เรื่อง loop-variable capture ทำไม directive ถึงสำคัญตรงนี้?

ไม่สำคัญเลย — bug นี้เหมือนกันทุก Go version Go 1.22 เปลี่ยนให้ตัวแปรใน for-loop มี scope แยกต่อ iteration ดังนั้นโค้ด capture แบบเดียวกันเป็น bug ก่อน 1.22 แต่ถูกต้องตั้งแต่ 1.22 ขึ้นไป go directive มีผลแค่กับว่า stdlib version ไหนใช้ได้เท่านั้น go directive เก่าๆ จะปิดการใช้งาน goroutine ไปเลย

ทำไม 'interface ใหม่มี implementation จริงมากกว่าหนึ่งตัวหรือเปล่า' ถึงอยู่ใน checklist นี้ด้วย ทั้งที่ไม่มี linter ตัวไหนเช็กเรื่องนี้ได้เลย?

ไม่ควรอยู่เลย — checklist ควรมีแค่สิ่งที่เครื่องมือ verify ได้เท่านั้น checklist มีไว้เพื่อการตัดสินใจที่ไม่มีเครื่องมือไหนทำได้พอดี — นั่นคือช่องว่างที่ human review มีไว้เติมเต็ม golangci-lint เช็กเรื่องนี้ได้ถ้าเปิด linter มากพอ interface ที่มี implementation แค่ตัวเดียวถือเป็น compile error เสมอ

Submit

Mark as complete