เหตุผลที่นักพัฒนาบางส่วนชอบรีวิวโค้ดแบบ interdiff

 (gist.github.com/thoughtpolice)
3 คะแนน โดย GN⁺ 2024-09-12 | 1 ความคิดเห็น | แชร์ทาง WhatsApp
  • การรีวิว PR แบบ GitHub ทำให้เพิ่มคอมมิตแก้ไขทับบนการเปลี่ยนแปลงเดิมได้ง่าย จึงเกิดปัญหาที่ กระบวนการรีวิว ปะปนกับประวัติโค้ดสุดท้าย
  • patch series ที่ดีควรอ่าน คอมมิตที่แยกตามตรรกะ ได้ตามลำดับ เช่น การ refactor, การเพิ่ม API ใหม่, และการย้ายผู้ใช้เดิมไปใช้ API ใหม่
  • เมื่อมีคอมมิตอย่าง “fix review” หรือ "minor" สะสมขึ้นมา เจตนาของการเปลี่ยนแปลงจะพร่ามัว และต้นทุนในการหาสาเหตุของปัญหาด้วย git blame กับ git bisect จะสูงขึ้น
  • การรีวิวแบบ interdiff คือการเผยแพร่ patch series เวอร์ชันใหม่ แล้วใช้ git range-diff main..v1 main..v2 เพื่อเปรียบเทียบความต่างของการเปลี่ยนแปลงในแต่ละคอมมิต
  • ผู้รีวิวสามารถตรวจเฉพาะส่วนเพิ่ม 50 บรรทัดได้โดยไม่ต้องอ่านทั้งหมด 500 บรรทัดใหม่ ส่วนผู้เขียนก็รักษาโครงสร้างคอมมิตสุดท้ายไว้ได้โดยไม่มีคอมมิตรก ๆ อย่าง “address review”

เครื่องมือรีวิวโค้ดและจุดตั้งต้น

  • Gerrit Code Review เป็น เครื่องมือรีวิวโค้ดโอเพนซอร์ส ที่ทำงานร่วมกับ Git repository รองรับเวิร์กโฟลว์การส่งแพตช์ ให้ผู้อื่นตรวจโค้ด และทิ้งคอมเมนต์
  • วิธีรีวิวโค้ดมีหลากหลาย ไม่ใช่แค่ Gerrit, GitHub และ Phabricator แต่รวมถึงการอัปโหลดไฟล์ .patch ไปยัง bug tracker, git send-email, หรือแม้แต่การถกเถียงยาว ๆ กับเพื่อนร่วมงานจนผลักดันให้เกิดการ implementation
  • ภาษาโปรแกรม Go ใช้ Gerrit, KDE และ LLVM เคยใช้ Phabricator ส่วน Jujutsu เคยพิจารณาใช้ Gerrit
  • Linux kernel ใช้เวิร์กโฟลว์บนอีเมลในรูปแบบเฉพาะ ขณะที่หลายโปรเจกต์เลือก GitHub เพราะใช้ง่ายและมีต้นทุนเริ่มต้นต่ำ
  • ประเด็นสำคัญคือเครื่องมือเหล่านี้ไม่ได้ให้โมเดลการรีวิวแบบเดียวกัน และประสบการณ์ใช้งานเริ่มต้นของ GitHub ก็ไม่ได้เพียงพอเสมอไป

เงื่อนไขของ patch series ที่ดี

  • patch series ในอุดมคติคือการส่งการเปลี่ยนแปลงขนาดใหญ่หนึ่งชุด โดยแบ่งเป็น ขั้นตอนเชิงตรรกะ
    • เริ่มจากจัดระเบียบโค้ดที่บังเอิญพบ
    • ต่อด้วยการเพิ่ม API ใหม่
    • สุดท้ายย้ายผู้ใช้ API เดิมไปใช้ API ใหม่
  • แต่ละแพตช์ควรมีเหตุผลที่เป็นอิสระ และเมื่อ apply ตามลำดับแล้วควรเห็นโค้ดค่อย ๆ วิวัฒน์ทีละขั้น
  • โครงสร้างแบบนี้เป็นประโยชน์ไม่เพียงต่อผู้เขียนและผู้รีวิว แต่ยังต่อ maintainer ที่ภายหลังต้องตามหาเหตุผลของการเปลี่ยนแปลงด้วย blame หรือ bisect
  • ขนาดแพตช์จริงขึ้นอยู่กับสถานการณ์
    • อาจไม่มีการ refactor เลยก็ได้
    • อาจมี 100 บรรทัดก็ได้
    • หากเป็นการเปลี่ยน API หลักที่ต้องแก้ทุกจุดที่เรียกใช้งาน ก็อาจมี 500 บรรทัดได้

ประวัติที่ปะปนจาก “diff soup” แบบ GitHub

  • GitHub ชักนำทั้งโดยชัดแจ้งและโดยนัยให้ใช้เวิร์กโฟลว์ที่ เพิ่มคอมมิตใหม่ ทับบนคอมมิตเดิมเมื่อแก้ตามคอมเมนต์รีวิว
  • ในตัวอย่าง หลังคอมมิตเดิม 3 ตัว จะมีคอมมิตที่แก้ตามรีวิวของ Alice และ Bob, เพิ่มการทดสอบ, และเปลี่ยน implementation แบบ "minor" สะสมต่อกัน
  • ใน commit graph จะเหลือเพียงความสัมพันธ์ parent-child จึงไม่เห็นว่าคอมมิตแก้รีวิวนั้นแก้ปัญหาส่วนไหนของคอมมิตเดิมใด
    • หากคอมมิต “fix review” หนึ่งตัวไปแตะคอมมิตเดิมหลายตัวพร้อมกัน โมเดลเชิงแนวคิดก็ยิ่งซับซ้อนขึ้น
    • สามารถบังคับให้คอมมิตแก้ไขหนึ่งตัวสะท้อนข้อทักท้วงเพียงหนึ่งรายการได้ แต่จำนวนคอมมิต fixup ก็จะเพิ่มขึ้นตามไปด้วย
  • รูปแบบที่เอาชุดการเปลี่ยนแปลงขนาดใหญ่กับการแก้ตามรีวิวมาคลุกอยู่ในชามเดียวกันแบบนี้คือ “diff soup”

สัญญาณของ git blame และ git bisect พร่ามัวลง

  • git blame ทำงานเป็นรายบรรทัด ดังนั้นแม้คอมมิตแก้ตามรีวิวจะเปลี่ยนเพียงส่วนเล็ก ๆ ของบรรทัดเดิม ต้นทางของบรรทัดนั้นก็อาจแสดงเป็นคอมมิตอย่าง "fix alice review"
  • หากต้องการหาเหตุผลแท้จริงของการเปลี่ยนแปลง จะต้องทำ โบราณคดีคอมมิต โดยไล่ตามทั้งคอมมิตเดิมและคอมมิตแก้รีวิวอีกครั้ง
  • ใน git bisect ก็อาจไม่ชัดเจนว่าคอมมิต "minor" คือสาเหตุจริง หรือเป็นบั๊กที่มีมาตั้งแต่ตอนเพิ่ม API ใหม่แล้วเพิ่งถูกเผยออกมาทีหลัง
  • หาก API ใหม่ให้ประสิทธิภาพดีขึ้นมาก การ revert อาจไม่พึงประสงค์หรืออาจทำไม่ได้
  • หาก regression ปรากฏขึ้นหลายสัปดาห์ให้หลัง ก็จะเกิดต้นทุนที่ต้องสืบต่อจาก commit message แค่ "minor" ในสถานการณ์เร่งด่วนอย่างการประชุม P1
  • แม้ในตัวอย่างที่รีวิวโต้ตอบกันเพียงสองรอบ ประวัติก็ซับซ้อนขึ้นแล้ว และในโปรเจกต์โอเพนซอร์สที่มีการรีวิวซ้ำหลายรอบ ปัญหาจะยิ่งใหญ่ขึ้น

วิธีที่ GitHub UX เสริมแรงให้เวิร์กโฟลว์นี้

  • บน GitHub วิธีที่ง่ายที่สุดในทางปฏิบัติสำหรับการรีวิวแบบ incremental คือเพิ่มคอมมิตใหม่ไว้ด้านบน
    • สำหรับผู้รีวิว การดูแพตช์ "fix alice review" ขนาด +/- 10 บรรทัด เบากว่าการอ่าน API ใหม่ 500 บรรทัดซ้ำ
  • แท็บ diff ของ PR โดยค่าเริ่มต้นจะแสดงคอมมิตทั้งหมดรวมกันเป็น diff เดียว
    • คล้ายกับการดู git diff master..foo-branch
    • ซึ่งไม่ค่อยสอดคล้องกับวิธีที่ผู้คนเขียนและอ่านการเปลี่ยนแปลง
  • GitHub UX ผูกกับโมเดล named branch อย่างแน่นหนา ทำให้ดูความต่างระหว่างหน่วยอื่นที่ไม่ใช่ branch ได้ยาก
  • หลัง force push สามารถดู diff ได้จากปุ่ม “Compare” ในแถว “Force Push” ของ PR
    • แต่จะแสดงเฉพาะ diff ทั้งหมด ระหว่าง head ของ branch ก่อนหน้าและ head ของ branch ปัจจุบัน
    • หากต้องการเปรียบเทียบละเอียดกว่านั้น ต้องหา commit ID เองแล้วใส่ใน URL

การรีวิวแบบ interdiff และ git range-diff

  • การรีวิวแบบ interdiff ไม่ได้เพิ่มคอมมิตแก้ไขทับบนคอมมิตเดิม 3 ตัว แต่เผยแพร่ เวอร์ชันใหม่ ที่ยังประกอบด้วยคอมมิต 3 ตัวเหมือนเดิม
  • ตัวอย่างเช่น หากต้องแก้ตามคอมเมนต์ของ Bob ใน patch series v1 ก็แก้คอมมิตแรกและคอมมิตที่สามเพื่อสร้าง series v2
  • ต่อให้ภายหลังแก้ตามคอมเมนต์ของ Alice, เพิ่มการทดสอบ และเปลี่ยนแปลงแบบ "minor" ด้วยวิธีเดียวกัน ผลลัพธ์สุดท้ายก็ยังคงเหลือ 3 คอมมิต
  • หากคอมมิต A, B, C ของ v1 วิวัฒน์เป็น X, Y, Z ของ v2 แล้ว git range-diff จะแสดงความต่างระหว่างคอมมิตในแต่ละตำแหน่ง
git range-diff \
  main..v1 \
  main..v2
  • คำสั่งนี้จะแสดง diff แบบจับคู่ ระหว่างคอมมิต 3 ตัวของ branch v1 กับคอมมิต 3 ตัวของ branch v2
    • คอมมิตแรกคือ diff(A, X)
    • คอมมิตที่สองคือ diff(B, Y)
    • คอมมิตที่สามคือ diff(C, Z)

ผลเชิงปฏิบัติของวิธี interdiff

  • ต่างจาก GitHub ที่มักแสดง diff ของทั้ง branch เช่น diff(main, C) หรือ diff(main, Z) interdiff แสดง ความต่างระหว่างเวอร์ชันของคอมมิต
  • ผู้รีวิวไม่ต้องอ่านการเปลี่ยนแปลง API ขนาด 500 บรรทัดที่เคยเห็นแล้วซ้ำ แต่ตรวจเฉพาะ 50 บรรทัดที่ถูกแก้ได้
  • ผู้เขียนไม่จำเป็นต้องสะสมคอมมิตรก ๆ อย่าง “address review” จำนวน 30 ตัวไว้ใน history
  • git blame มีแนวโน้มเชื่อมโยงบรรทัดไปยังคอมมิตเปลี่ยนแปลงเดิมได้โดยมี noise น้อยลง
  • git bisect มีแนวโน้มชี้ regression ไปยังหน่วยที่มีความหมายกว่า เช่น คอมมิตเพิ่ม API ใหม่
  • ผลคือ อัตราส่วนสัญญาณต่อสัญญาณรบกวน ของเครื่องมือวินิจฉัยพื้นฐานดีขึ้น

หมายเหตุเรื่องกลยุทธ์การ merge และ rebase

  • คำอธิบายเรื่องกลยุทธ์การ merge แพตช์ยังเหลือเป็น TODO
  • มีจุดยืนว่า git rebase ใช้ได้ ตราบใดที่ไม่ใช่ public branch ที่คาดว่าจะมีคนอื่นสร้างคอมมิตต่อยอดไว้
  • ระบบรีวิวแบบ interdiff ส่งเสริมแพตช์ที่เล็กลงและ merge ได้เร็วขึ้น
    • ไม่จำเป็นต้องรอจนคอมมิตทั้ง 5 ตัวพร้อมทั้งหมด
    • หาก 3 ตัวแรกใช้ได้แล้ว แต่ 2 ตัวหลังยังต้องปรับเพิ่ม ก็ merge เฉพาะ 3 ตัวได้
  • เป้าหมายคือทำให้ผู้คนทำงานโดยอิงกับ branch main แทนที่จะใช้ long-running branch เพื่อลดสถานการณ์ที่ remote branch หลายตัวเกี่ยวพันและถูก merge เข้าหากัน
  • มีโปรเจกต์อย่าง Linux kernel ที่ merge public branch กับ repository อื่น หรือประกาศใช้ public branch ไว้บน branch อื่นอย่างชัดเจน
    • นักพัฒนา Linux สามารถใช้ git rebase ตอนสร้างและขัดเกลา patch series ได้
    • แต่หลังจากสร้าง public branch แล้ว เนื่องจากอาจถูก merge เข้า tree ของใครบางคนได้ จึงจะไม่ force push แต่สร้าง branch ใหม่ที่มีคอมมิตใหม่แทน

บทความที่เกี่ยวข้อง

1 ความคิดเห็น

 
GN⁺ 2024-09-12
ความคิดเห็นบน Hacker News

  • โดยทั่วไปผมใช้ flow แบบนี้บน GitHub อยู่ ข้อเสียคือผมต้องทำงานมากขึ้น และมันไม่ค่อยเข้าใจง่ายสำหรับคนที่ร่วมงานด้วย
    ถึงอย่างนั้น ข้อดีที่ reviewer สามารถดู diff ที่สะท้อนเฉพาะ feedback ของตัวเองได้ และไม่ทำให้ git blame กับ git bisect พัง ก็ยังคงอยู่
    ตอนนำ feedback จาก review มาปรับ ผมจะ commit ด้วย git commit --fixup แล้ว push จากนั้นทิ้ง hash ของ fixup commit ไว้ใน reply ของ comment review
    เมื่อ PR ได้รับ approval และใกล้จะ merge ผมจะรัน git rebase --interactive origin/main --autosquash เพื่อรวม fixup commit เข้ากับ commit เดิม แล้วสุดท้าย git push --force-with-lease ก่อน merge
    ต้องระวังการ force push ก่อน review จบ เพราะถ้าทำแบบนั้น reviewer จะไม่สามารถเห็นสิ่งที่เพิ่มเข้ามาหลัง review ครั้งล่าสุดได้
    ผมพึ่งพา autocomplete ใน terminal มาก แค่พิมพ์ประมาณ git re ก็ไปถึงคำสั่งยาว ๆ ได้แล้ว แต่มันยังค่อนข้างเก้งก้างอยู่ ถ้ามีเครื่องมือที่รองรับและส่งเสริม flow นี้ก็คงดี ถึงอย่างนั้นถ้าติดอยู่กับ GitHub แค่นี้ก็ถือว่าโอเค

    • ผมก็กำลังจะพูดแบบนั้นเหมือนกัน interactive rebase ช่วยแก้ปัญหา “diff soup” ที่ผู้เขียนบ่นได้ค่อนข้างมาก
      แต่ในทางปฏิบัติมันมักถูกใช้ในทีมวิศวกรรมที่มีวินัยและตั้งใจเรียนรู้ฟีเจอร์ขั้นสูงของ git เท่านั้น
    • ใส่ config ด้านล่างไว้ใน ~/.gitconfig ก็พอ
      [rebase] autosquash = true
      จากนั้นแค่ git rebase -i origin/main ในภายหลัง fixup/squash commit ก็จะถูกจัดเรียงใหม่ให้อัตโนมัติ เป็น config เล็ก ๆ แต่ทำให้ workflow ดีขึ้นมาก
    • https://news.ycombinator.com/item?id=37086022
    • fixup commit ก็เป็นแนวทางที่ดี แต่ส่วนตัวผมไม่ค่อยชอบ ผมมองว่าวิธีแบบ คำสั่ง absorb ของ Sapling ที่ดูดซับการเปลี่ยนแปลงเข้าไปใน diff ที่เกี่ยวข้องโดยอัตโนมัติบน SCCS weave ภายใน แล้วให้ตรวจยืนยันผ่าน UI แบบ interactive นั้นสง่างามกว่ามาก
      มันอยู่ในพื้นที่คล้ายกับกรณีที่ต้องใช้ --update-refs ตอน rebase ชุด commit ที่มีหลาย branch วางซ้อนอยู่ ทำไมมนุษย์ต้องคอยไล่ความสัมพันธ์ของ graph ใส่ commit เอง และย้าย branch เองด้วย คอมพิวเตอร์จัดการ graph ได้ดีอยู่แล้ว ก็ให้มันทำไป
      git absorb ก็มีเหมือนกัน แต่ยังไม่แข็งแรงเท่า implementation ของ Sapling[1]
      ปัญหาจริง ๆ ไม่ใช่ว่าจะใช้ interactive rebase หรือไม่ แต่โดยมากเป็นเรื่อง ประสบการณ์ผู้ใช้ ของเครื่องมือ review เอง และวงจรการทำงานที่มันชักนำให้เกิด ตัวอย่างเช่น fixup commit แก้ปัญหาที่ GitHub แสดง diff ระหว่างจุดอ้างอิงไม่ได้ และถ้าจุดอ้างอิงใหญ่ ก็อาจทำให้ review พังทั้งหมดได้ เช่นกรณี rebase ขึ้นไปบน commit ใหม่ 10 ตัว
      ผมก็ไม่พอใจ UX ของ Git เองเหมือนกัน แต่บทความต้นทางดูจะเป็นการบ่น GitHub เป็นหลักมากกว่า
      [1] ตัวอย่างที่แสดงความแตกต่างของอัลกอริทึมทั้งสองฐานอยู่ใน GitHub issue นี้: https://github.com/martinvonz/jj/issues/170
    • Git alias นี่เหมือนเวทมนตร์จริง ๆ

  • เห็นด้วย 100% ว่าวิธีนี้คืออุดมคติ วิธีของ GitHub แย่มากจริง ๆ และเป็นเรื่องน่าเศร้าที่คนจำนวนมากเกินไปยอมรับมันเป็นเรื่องปกติไปแล้ว
    สมัยก่อนเราเคยทำแบบนี้ด้วย Phabricator และพอทำแบบ manual ได้ในระดับหนึ่งด้วย command-line macro ที่อัปเดต review ทั้งหมดในครั้งเดียว ถึงอย่างนั้น ถ้ามี UI ที่ชัดเจนกว่านี้ก็คงดี

    • ผมเป็นผู้เขียนบทความ และผมตั้งใจใช้วลีช่วงต้นว่า “code review โดยทั่วไปแล้วเป็นไอเดียที่ค่อนข้างดี” เพราะมันเคยเป็นหนึ่งในข้อความขายของบนหน้าเว็บ Phabricator เมื่อก่อน :) คิดถึงจัง

  • ใช่เลย สิ่งที่ผมนึกภาพไว้ในหัวว่าเป็นสไตล์ code review ของจริงก็คือแบบนี้ ไม่ใช่วิธีที่ GitHub ทำ ดีใจที่ได้รู้ว่ามันมีชื่อเรียก
    เพิ่มเติมคือ ถ้าระบบ review สามารถดัน patch ที่พร้อมแล้ว “ออกจาก” review ได้ก็คงดี bug fix เล็ก ๆ ที่เกิดขึ้นระหว่างทำ feature ใหญ่ควรเป็น patch เล็ก ๆ ที่แยกอิสระ และมีโอกาสสูงที่จะตกลงกับ reviewer ได้เร็ว
    ถ้าเป็นแบบนั้น ผมอยากดึง patch นั้นออกจาก series ทั้งหมดแล้ว cherry-pick เข้า main จากนั้น rebase review ขึ้นบน HEAD ใหม่ หรืออาจ rebase patch series ล่าสุดขึ้นบน main แต่จัดเรียง patch ที่ตกลงกันแล้วให้อยู่หน้าสุด แล้ว fast-forward main ไปยัง patch นั้นก็ได้
    โดยแก่นแล้วคืออยากจำกัดขอบเขต review ให้เหลือแค่ “ส่วนที่ยังถกเถียงกันอยู่” แต่ให้ bug fix ถูก merge ทันทีที่พร้อม
    ข้อโต้แย้งก็คงเป็น “ก็ทำเป็น review/PR แยกสิ” แต่แบบนั้นจะเกิดความซับซ้อนแบบ patchset A พึ่งพา patchset B แล้วหลังจาก B ถูก merge ก็เปลี่ยนไปพึ่งพา main แทน

    • พูดซ้ำแล้วซ้ำอีกว่า Gerrit ทำแบบนั้นให้โดยพื้นฐานอยู่แล้ว :) ใน Gerrit คุณดูความสัมพันธ์ระหว่าง patch สองตัวใด ๆ ก็ได้ และที่สำคัญกว่านั้นคือมันแสดงแต่ละ patch แยกกัน ดังนั้นใน series A -> B -> C ถ้า B เล็กและโอเค ก็เอาเข้าได้เลย
      ตรงนี้ยังมีไอเดีย UX ฉลาด ๆ อย่าง Attention Set ช่วยด้วย โดยพื้นฐานมันบอกว่า “ใครคือคนที่ต้องทำ action ถัดไป?” และทำงานเหมือนเกมผลัดตา ถ้าคุณเพิ่ง review ไป คุณก็จะไม่อยู่ใน attention set ของ patch นั้นแล้ว แต่ผู้เขียนจะอยู่แทน
      ดังนั้นใน UI ของ Gerrit รายการนั้นจะเลื่อนลงไปอยู่ด้านล่างของ queue ผม ส่วนด้านบนสุดของ queue คือรายการที่มีผมอยู่ใน attention set งานจึงถูกจัดกลุ่มไปในลักษณะนี้อย่างเป็นธรรมชาติ
      ยังไม่ได้พูดถึงปัญหาจุกจิกน่ารำคาญอื่น ๆ ของ UX GitHub ทั้งหมดด้วยซ้ำ แค่รายการ pull request ก็แย่กว่าทางเลือกอื่นแล้ว ดูไม่ออกว่ารายการไหนอยู่สถานะใด สุดท้ายต้องอ่านทั้งหมด
    • Gerrit ทำสิ่งนั้นได้พอดีเลย ถ้า push chain x-b-c-d-e เข้าไป UI จะแสดงเป็นแบบซ้อนกัน แต่คุณสามารถ cherry-pick b เข้า main ได้ง่าย ๆ ตรวจว่า CI ผ่านและ review ปกติเรียบร้อย แล้วค่อย rebase ส่วนที่เหลือขึ้นไปบนมัน
      ถ้าเป็น x ซึ่งอยู่ล่างสุด ก็ submit ได้ทันทีแล้วดำเนินการส่วนที่เหลือต่อ

  • การได้เห็นแนวทางใหม่ต่อการรีวิวโค้ดเป็นเรื่องน่าสนใจเสมอ GitHub ก็มีข้อดี แต่ยังห่างไกลจากความสมบูรณ์แบบ
    ถ้าเป็นสถานการณ์ที่ยกมา อาจพิจารณาแยกแพตช์ทั้งสามออกเป็น pull request แยกกันที่พึ่งพากันได้ แม้ GitHub จะไม่รองรับโดยตรง แต่เครื่องมือรีวิวโค้ดที่เหมาะสมควรจัดการ dependency ได้ในขณะที่ยังทำให้ pull request มีขนาดเล็ก เช่น ให้แพตช์ 3 พึ่งพาแพตช์ 2 และแพตช์ 2 พึ่งพาแพตช์ 1
    ด้วยการติดตาม dependency ที่เครื่องมือมีให้ ก็สามารถรับประกันได้ว่าหากจำเป็น ทั้งหมดจะถูก deploy ไปพร้อมกัน
    การรีวิวแต่ละแพตช์แยกกันทำให้ feedback ชัดเจนขึ้นและตอบสนองได้ง่ายขึ้น ภายใน pull request ยังสามารถ squash commit เพื่อสร้างประวัติ commit ที่สะอาดและสะท้อนการเปลี่ยนแปลงแต่ละส่วนได้อย่างถูกต้อง ยิ่งไปกว่านั้น หากเป็นเครื่องมือที่เหมาะสม ก็สามารถใช้ AI สรุป pull request และรีวิว เพื่อทำให้การเขียน commit message ที่ถูกต้องง่ายขึ้นโดยไม่ต้องทำมือ
    เครื่องมือรีวิวโค้ดที่ดีไม่ควรถูกกระทบจากงาน git อย่าง rebase, merge หรือ force push ไม่ว่าข้างหลังจะมีงาน git ซับซ้อนแค่ไหน reviewer ควรเห็นเฉพาะสิ่งที่เปลี่ยนไปหลังจากการรีวิวครั้งล่าสุดเท่านั้น แบบนั้นจะทำให้โฟกัสกับเนื้อหาใหม่ได้โดยไม่ต้องรีวิว diff ขนาดใหญ่อีกครั้ง และประวัติการรีวิวก็จะแยกจากประวัติ commit และคงความสะอาดไว้ได้
    สงสัยว่าวิธีแบ่ง pull request และติดตาม dependency ระหว่างกันแบบนี้จะตอบโจทย์หรือไม่

    • คำว่า “แนวทางใหม่ต่อการรีวิวโค้ด” เป็นคำที่ฟังบวกและสุภาพต่อผู้เล่นเดิมที่แข็งแกร่ง แต่ไอเดียนี้ไม่ได้ใหม่เลย
      พูดตรง ๆ คือมันเป็น workflow ที่ git ถูกออกแบบมาให้รองรับ และเป็นแก่นของคำวิจารณ์เก่าแก่ที่มีมาตลอดตั้งแต่ GitHub มี pull request
      ผมสงสัยว่าทำไมไอเดียนี้ถึงถูกดูแคลนว่าเป็นวิธีของ “พวกตกยุคจาก mailing list” มานานกว่า 15 ปี พอดูปฏิทินแล้ว แล้วเพิ่งไต่ขึ้นมาเป็นกระแสในวัฒนธรรมนักพัฒนาได้ตอนนี้
      ผมคิดมานานแล้วว่านักพัฒนาซอฟต์แวร์จำนวนมากจงใจเลือก workflow ที่แย่ เพราะปฏิเสธที่จะเรียนรู้เครื่องมือหนึ่งในเครื่องมือที่พวกเขาใช้บ่อยที่สุดอย่างจริงจัง ดังนั้นตอนย้ายบริษัทไปใช้ git ผมจึงเลือกและสร้างเครื่องมือกับ workflow แบบนั้นขึ้นมา อย่างไรเสีย ซื้อ IBM ก็ไม่โดนไล่ออกนี่นะ
    • เท่าที่ผมรู้ วิธีแยก series ออกเป็น PR แยก ๆ จะทำงานได้ดีต่อเมื่อคุณมีสิทธิ์ commit ใน repository เท่านั้น เพื่อให้เอา PR หนึ่งไปวางบน branch อื่นของ repository หลัก แทนที่จะวางบน main ได้
      ถ้า contributor ภายนอกใช้ fork ของ repository PR ทั้งสามจะค่อย ๆ รวมเป็น A, A+B, A+B+C ตามลำดับ แบบนั้น PR สองอันหลังจะยังต้องเห็นโค้ดที่รีวิวไปแล้วใน PR อื่นเป็น diff อีก ทำให้รีวิวยาก
    • ผู้เขียนมี branch และ commit ของมันอยู่แล้ว ถ้าอย่างนั้นจะแยก branch ออกเป็น pull request สามอันที่แต่ละอันมี commit เดียวไปเพื่ออะไร?
      แต่ละ commit สามารถรีวิวแยกกันได้ สามารถติดตาม dependency ระหว่าง commit ได้ด้วย และข้อดีอีกอย่างคือไม่มี AI ฝังอยู่ ต่างจาก PR ยังสามารถดู interdiff ระหว่าง commit ได้ด้วย
      commit คือวัตถุดิบพื้นฐานของ Git ก็แค่ใช้ commit ไปเลย
    • อย่างที่มีพูดถึงในที่อื่นของเธรดนี้ นี่ก็เป็นแนวทางที่ Sapling ใช้เช่นกัน
      สงสัยว่า GitContext ติดตาม commit ที่ผ่านงานอย่าง fixup, rebase หรือการจัดลำดับใหม่อย่างไร
    • ใช้วิธี merge train แบบเก่า ๆ เฉย ๆ ไม่ได้หรือ? ให้ pull request A ชี้ไปที่ branch B และให้ B ชี้ไปที่ master จากนั้น merge B เข้า master แล้วค่อยปรับ A กลับมาที่ master อีกที ก็พอไม่ใช่หรือ ผมพลาดประเด็นอะไรไปหรือเปล่า?

  • ผมเป็นคนเขียนบทความนี้เอง ถามอะไรก็ได้
    เพิ่มเติมคือ ไม่แน่ใจว่าผิดกฎหรือเปล่า แต่ช่วงนี้ผมต้องหางานใหม่ ชอบทำงานกับเครื่องมือสำหรับนักพัฒนาและปัญหายาก ๆ ถ้าชอบบทความนี้ หรืออยากเพิ่ม productivity ให้ทีมพัฒนา หรืออยากสัมผัสและสนุกกับรสนิยมอันยอดเยี่ยมและบางครั้งก็ eclectic ของผม อีเมลอยู่ในโปรไฟล์

    • ขอบคุณที่ทำให้หัวเราะได้ดี :)
      พูดจริง ๆ แล้ว เรซูเม่น่าประทับใจมาก ขอให้ได้ที่ลงดี ๆ และเร็ว ๆ จากประสบการณ์หางานสด ๆ ร้อน ๆ ของผม ตลาดจ้างงานตอนนี้ป่วยหนัก ยิ่ง senior และมีประสบการณ์มากเท่าไร กระบวนการสัมภาษณ์/HR ที่บ้าบอและการปฏิเสธแบบไร้คำอธิบายก็ยิ่งทำร้ายจิตใจมากขึ้นเท่านั้น
      เพื่อนส่งลิงก์ด้านล่างมาให้
      https://danluu.com/hiring-lemons/
      https://danluu.com/programmer-moneyball/
      https://danluu.com/algorithms-interviews/
      ขอให้โชคดี
    • สรุปโมเดลความคิดของ สไตล์การรีวิวแบบ interdiff และปัญหาของแนวทางรีวิวโค้ดของ GitHub ได้ดีมาก
      สิ่งหนึ่งที่ยังไม่ได้กล่าวถึงคือ เพราะ diff soup อาจทำให้คนชอบกลยุทธ์ squash merge เพื่อกำจัด “noise” ของ fixup commit แล้วก็ทิ้ง commit แบบ atomic ที่ดี 3 อันแรกไปพร้อมกันด้วย
      ในสไตล์การรีวิวแบบ interdiff commit 3 อันแรกจะยังคงอยู่ และจะเลือกนำเข้าแยกกันหรือ squash ก็ขึ้นอยู่กับตัว commit เองและความเป็น atomic จริง ๆ ของมันทั้งหมด
    • จากประสบการณ์ใช้ส่วนฟรีของ GitHub คำว่า diff soup เหมาะมาก เวอร์ชันเสียเงินดีกว่านี้ไหม? GitLab เข้าใกล้ Gerrit ได้หรือเปล่า?
      ยังมีบริการภายนอกที่พยายามลดความเจ็บปวดของ GitHub และให้การรองรับ stacked diff ด้วย แต่ดูค่อนข้างแพง โดยเฉพาะเมื่อเทียบกับ Gerrit แบบ self-hosted สงสัยว่าได้ลองดูเครื่องมือพวกนี้ด้วยไหม
    • อยากรู้ว่าคิดอย่างไรกับ GitLab ที่ตั้งค่า Merge Request Dependencies และกลยุทธ์ merge แบบ Squash+Merge
      จำได้ว่าเมื่อก่อนตั้ง pattern ของ MR series ได้ค่อนข้างง่าย เมื่อ merge MR 3 อัน ก็จะได้ merge commit 3 อัน โดยแต่ละ MR มี squash commit เดียว ไม่เกี่ยวกับประวัติ commit ของ MR
      trade-off คือถ้า branch ช่วงต้นของ series เปลี่ยนระหว่างรีวิว ก็ต้อง merge เข้า branch ช่วงท้ายของ series ใหม่
      ถึงอย่างนั้น ระหว่างรีวิว ผู้คนก็จัดการประวัติ commit ได้ตามต้องการ เพราะสุดท้ายก็ squash อยู่ดี จึงไม่เป็นไร
      แต่ก็ไม่ได้ทำมานานแล้ว และช่วงนี้ติดอยู่กับ GitHub เลยอาจกำลังมองด้วยความทรงจำที่สวยงามเกินจริงก็ได้

  • Review Board [https://www.reviewboard.org] ได้สร้าง interdiff ไว้ตั้งแต่ปี 2006 แล้ว จริง ๆ แล้วคำนี้อาจเป็นผมเองที่บัญญัติขึ้น หรือไม่ก็ไปถึงแนวคิดนี้ได้อย่างเป็นอิสระ
    จนถึงตอนนี้มันยังเป็นส่วนที่ผมชอบที่สุดของผลิตภัณฑ์และกระบวนการรีวิวโค้ด และเป็นหนึ่งในฟีเจอร์ที่ได้ยินบ่อยว่าผู้คนคิดถึงมากที่สุดเมื่อย้ายไปใช้สิ่งอย่าง GitHub
    ผมไม่เคยรู้สึกว่า commit แบบ fix-it เป็นทางเลือกทดแทนที่เหมาะสมเลย
    อย่างแรก มันไม่ได้บอกว่าการเปลี่ยนแปลงในชั้นบนถูกสะท้อนเข้าไปในชุด commit อย่างไร
    อย่างที่สอง มันทำให้กราฟ commit รก แม้จะชั่วคราวก็ตาม และทำให้รีวิวยากขึ้น คนที่ติดตามการรีวิวมาตลอดอาจอ่านโค้ดที่ commit fix-it นั้นแก้ไปแล้ว แต่คนที่เพิ่งเข้ามาใหม่อาจเริ่มด้วยความประทับใจแรกที่ไม่ดีว่าโค้ดนั้นพยายามทำอะไรหรือมีโครงสร้างอย่างไร
    อย่างที่สาม ไม่ใช่ทุกคนจะใช้ Git หรือระบบจัดการเวอร์ชันที่รองรับหลาย commit ได้ ในการพัฒนาเกมมักใช้ Perforce ส่วนผู้ผลิตชิปก็มักใช้ระบบจัดการเวอร์ชันเฉพาะทางอย่าง Keysight SOS ในที่แบบนั้น commit แบบ fix-it เองไม่ใช่ตัวเลือกด้วยซ้ำ
    ถ้าเป็นระบบรีวิวโค้ดที่รองรับ interdiff อย่างเหมาะสม reviewer คนหนึ่งสามารถติดตามตั้งแต่คำขอรีวิวที่เผยแพร่ครั้งแรกไปจนถึงทุกอัปเดต โดยดูเฉพาะสิ่งที่เปลี่ยนไปได้ ขณะที่ reviewer อีกคนสามารถเข้าไปดูการเปลี่ยนแปลงล่าสุดทั้งหมดได้ทันที โดยไม่ต้องสนใจชุด fix-it และสิ่งนี้ทำได้โดยไม่ขึ้นกับระบบจัดการเวอร์ชัน
    ถ้าทำให้ดี มันก็อยู่ร่วมกับการเปลี่ยนแปลงที่ประกอบด้วยหลาย commit ได้ด้วย
    เช่น เราอาจแบ่งโปรเจกต์เล็ก ๆ ออกเป็นหลาย commit เช่น API handler, UI ฝั่ง frontend, และเอกสาร เพื่อช่วยในการรีวิว และตัดสินว่า commit เหล่านั้นเกี่ยวข้องกันอย่างใกล้ชิด จึงเหมาะจะส่งขึ้นเป็นคำขอรีวิวเดียวกัน หากเกี่ยวข้องกันน้อย ก็คงเหมาะกว่าที่จะสร้างคำขอรีวิวหลายรายการเป็น dependency chain
    ตาม feedback จากการรีวิว เราอาจสร้างชุดการเปลี่ยนแปลงกับ commit หนึ่งรายการหรือทั้งหมดในกลุ่มนั้นได้ เวลาผู้คนรีวิวอัปเดต คงดีกว่าหากพวกเขาเห็นได้ว่าแต่ละส่วนวิวัฒน์ไปอย่างไร แทนที่จะต้อง mapping ในหัวระหว่าง commit fix-it กับการเปลี่ยนแปลงเดิมที่สอดคล้องกัน
    ดังนั้น interdiff จึงยอดเยี่ยมจริง ๆ ไม่ว่าจะมี commit เล็ก ๆ จำนวนมากหรือ commit ใหญ่ ๆ จะเป็นคำขอรีวิว commit เดียวหรือหลาย commit ก็ตาม ควรมีคนใช้มันมากกว่านี้

    • สุดยอดจริง ๆ อันที่จริงในตอนที่ 2 ของบทความนี้ ผมตั้งใจจะพูดถึงประวัติศาสตร์และบริบททางการเมืองที่ทำให้มาถึงเครื่องมือเหล่านี้
      ราวปี 2013 Glasgow Haskell Compiler ย้ายจากวิธีที่พูดเล่น ๆ ว่า “อ่านไฟล์ .patch จาก bug reporter” ไปใช้ Phabricator มีเหตุผลอยู่หลายข้อ แต่ตอนนั้นไม่ใช่เพราะ stacked diff เพราะ GitHub แย่มากสำหรับการรีวิว ถึงขั้นไม่มีแม้แต่ side-by-side diff ด้วยเหตุผลแค่นั้นก็หลุดจากการพิจารณาไปโดยสิ้นเชิงแล้ว และยังมีเหตุผลอื่นอีก
      แต่ครั้งนั้นไม่ใช่ครั้งแรกที่ผมย้ายทีมไปใช้เครื่องมือรีวิวโค้ด งานแรกของผมในปี 2009 เป็นทีมวิศวกรเล็กมากและสนิทกันมาก อยู่รวมกันในห้องหนึ่งที่ Houston และผมคิดว่าถ้าให้คนอื่นรีวิวโค้ดของผม และผมอ่านสิ่งที่พวกเขาเขียน ผมจะเข้าใจ codebase ได้ดีขึ้น
      ดังนั้นในช่วงสองสามเดือนแรกหลังเข้าทำงาน ผมจึงตื๊อผู้จัดการจนได้ติดตั้ง ReviewBoard และทุกคนก็ชอบมันมากจริง ๆ
      เพราะฉะนั้นนี่คือคำขอบคุณถึง RB ผมยังนึกถึงมันด้วยความทรงจำดี ๆ เป็นครั้งคราว ต้องขอบคุณสิ่งนั้น การรีวิวโค้ดจึงเป็นส่วนสำคัญมากในอาชีพของผมแทบตั้งแต่วันแรก และตอนนี้ผมก็ยังคิดว่าน่าจะทำได้มากกว่านี้

  • โดยรวมแล้ว ผมรู้สึกว่าระบบอย่าง Gerrit ที่ให้ความสำคัญกับการรีวิวโค้ดก่อนและเน้น rebase เป็นศูนย์กลาง ทำให้โค้ดรีวิวได้ง่ายกว่ามาก
    หนึ่งในข้อดีที่สุดคือการรองรับการซ้อน patch หลายชั้นเป็นค่าเริ่มต้น ซึ่งทำให้ผู้คนสร้าง patch ที่เล็กลงและรีวิวง่ายขึ้น
    การรีวิวโค้ดของ GitHub รู้สึกเหมือนฟีเจอร์ที่ถูกเพิ่มตามมาทีหลังแบบไม่ดีนัก ทั้ง UI ที่เปลืองพื้นที่และดูเหมือน thread ในฟอรัม รวมถึงการติดตามข้าม rebase ไม่ได้ เป็นต้น

    • มีสิ่งหนึ่งที่น่าเสียดายเวลา push stack ของ commit ใน Gerrit คือไม่มีพื้นที่กลางสำหรับพูดคุยเกี่ยวกับ stack ทั้งหมด แทนที่จะเป็น commit รายตัว
      การสนทนาที่เป็น “ภาพใหญ่” แต่ก็ยังเป็นเชิงเทคนิคแบบนี้เกิดขึ้นใน issue tracker บ่อยเกินไป แต่ผมก็ไม่รู้เหมือนกันว่าควรวางไว้ที่ไหน stack นี้ชั่วคราวเกินไป และอาจเปลี่ยนไปโดยสิ้นเชิงในการ push ครั้งถัดไป

  • เคยได้ยินคนสนับสนุนกลยุทธ์แบบนี้อยู่หลายครั้ง แต่ก็ยังไม่ถูกโน้มน้าว โปรเจกต์ส่วนใหญ่ที่ผมทำงานด้วยจะ squash feature branch ให้เป็นคอมมิตเดียวแล้ว merge จากนั้นลบประวัติของ branch ทิ้ง
    ถ้ามีกรณีแบบที่ผู้เขียนอธิบาย ผมคงทำให้ refactoring, API ใหม่, และการอัปเดตเป็น 3 คอมมิตไปเลย
    สิ่งที่เป็นแนวปฏิบัติที่ดีอย่างชัดเจนสำหรับผมคือการหลีกเลี่ยง branch ที่มีอายุยาว สถานการณ์คอมมิตหลายขั้นแบบนี้มักเกิดจากตรงนั้น คือมีคนทำฟีเจอร์อยู่หลายวัน หรือหนักหน่อยก็หลายสัปดาห์ แล้วอยากโยนเข้ามาทีเดียว
    ผมชอบย้ายเข้า main ทุกวัน หรือแม้แต่วันละหลายครั้งมากกว่า วิธีหนึ่งในการทำแบบนี้คือใช้ feature flag เพื่อให้คอมมิตที่ยังอยู่ระหว่างทำสามารถถูกรวมเข้าไปได้โดยไม่มีปัญหา ถ้าระบบถูกจัดวางอย่างเหมาะสม ก็มีข้อดีตรงที่สามารถเปิดในสภาพแวดล้อม development/staging เพื่อทดสอบได้ด้วย และยังห่างจากการ deploy แบบ blue/green แค่ก้าวเดียว
    ผมไม่ได้ต้องการวิธีรีวิวคอมมิตใหญ่ ๆ ให้ง่ายขึ้น แต่อยากบังคับให้ทีมคอมมิตการเปลี่ยนแปลงเล็ก ๆ ตั้งแต่เนิ่น ๆ และบ่อย ๆ มากกว่า ผมเข้าใจว่าไม่ใช่ทุกคนจะเห็นด้วย

    • คนที่ชอบ stacked diff บอกว่ามันทำให้รวมการเปลี่ยนแปลงได้เร็วขึ้น ไม่ว่าขนาดจะเป็นอย่างไร
      เหตุผลหนึ่งคือ ต่างจาก PR ของ GitHub ตรงที่สามารถรวมเฉพาะบางส่วนของ stack ได้ ถ้าดูจากตัวอย่างในบทความ หาก diff “refactoring เล็ก ๆ” พร้อมแล้ว ก็สามารถใส่มันเข้าไปก่อนได้โดยไม่ต้องรวม diff “API ใหม่” และ “ย้ายผู้ใช้ API” ไปพร้อมกัน
      ถ้าวางศูนย์กลางไว้ที่ คอมมิต แทนที่จะเป็น branch ก็มีผลทำให้คอมมิตโดยรวมเล็กลงกว่าการใช้ branch ที่มีอายุยาว
    • การ squash feature branch ให้เป็นคอมมิตเดียวแล้วลบประวัติของ branch ทิ้งนั้นแย่มาก
      แบบนั้นทำให้ git blame และ git bisect แทบใช้ประโยชน์ไม่ได้
      ถ้ามี regression git bisect สามารถไล่แคบลงไปจนถึงแพตช์เดียวได้ ดังนั้นสำหรับฟีเจอร์หนึ่ง ๆ การมีแพตช์ขนาด 160 บรรทัด 50 ชิ้นอยู่ในประวัติ git ย่อมดีกว่ามีแพตช์เดียวขนาด 8000 บรรทัด
      ถ้าโค้ดบรรทัดหนึ่งดูน่าสงสัย ผมอยากให้ git blame หรือคำสั่ง git blame ต่อเนื่องพาไปยังคอมมิตขนาด 160 บรรทัดที่มีข้อความคอมมิตละเอียด ไม่ใช่คอมมิตขนาด 8000 บรรทัด
      ควรรักษาลำดับของคอมมิตดั้งเดิมไว้ด้วย หลายปีให้หลัง แค่ได้อ่านข้อความคอมมิตแต่ละอันตามลำดับก็อาจช่วยให้เข้าใจการออกแบบเดิมได้มาก แน่นอนว่า patchset เดิมต้องจัดตามลำดับ dependency และต้องคอมไพล์ได้ในแต่ละขั้น เป็นต้น ซึ่งต้องมีขั้นตอนการพัฒนาแยกต่างหากอยู่เหนือการ implement ฟีเจอร์ โค้ดควรถูกนำเสนอเป็นขั้นตอนเชิงตรรกะ

  • น่าสนใจ ที่ทำงานเราใช้ PR ในแบบที่ผู้เขียนใช้คอมมิต และสุดท้ายก็ squash-and-merge จริง ๆ
    แต่แนวทางนี้ทำให้ทุกครั้งที่แก้ PR ด้านหน้า ต้อง rebase PR ด้านหลังตามไปด้วย ค่อนข้างยุ่งยาก แถมยังชนกับกฎ “ห้าม force-push” ใช้เวลานานกว่าวิศวกรจะเรียนรู้ และมีแนวโน้มทำให้คอมเมนต์ code review เดิมใน GitHub UI พังด้วย ถึงอย่างนั้น ถ้าเป็นสักสองสาม PR ก็ยังพอทำงานได้
    ใน flow ของเรา คอมมิตใกล้เคียงกับจุดบันทึกมากกว่าจะเป็นหน่วยงาน
    เราใช้ stash เยอะ และก็ใช้ undo-tree-mode ของ Emacs เยอะด้วย แบบนี้วิธีติดตามประวัติซอร์สโค้ดจึงมีสี่แบบ ซึ่งฟังดูซ้ำซ้อน แต่ในทางปฏิบัติก็ทำงานได้ดี
    ความใช้งานง่าย ในการจัดการเรื่องนี้บน Git ค่อนข้างแย่ Phabricator ดีกว่า แต่ก็ยังยากโดยไม่จำเป็น บางทีเครื่องมือจัดการเวอร์ชันใหม่อาจรองรับแนวคิดระดับสูงกว่าคอมมิตและ branch ให้เป็น first-class ได้ หรือบางทีอาจทำให้ใช้งานแย่ลงกว่าเดิมก็ได้

    • กำลังใช้คอมมิตเหมือน PR อยู่ เลยทำให้งานซับซ้อนเพราะ dependency ระหว่าง PR
      ถามว่าอยากได้เครื่องมือที่ดีกว่า Git ไหม? แน่นอน ผมก็อยากได้เครื่องมือที่มีข้อบกพร่องน้อยกว่าและเป็นมิตรกับผู้ใช้มากกว่า แต่ถ้าเป็นวิธีแก้ขัด ลองปฏิบัติกับคอมมิตเหมือนคอมมิตดูไหม? ไม่ต้องทำให้ตัวเองลำบากโดยไม่จำเป็น
      ผมสงสัยว่า GitHub มีส่วนรับผิดชอบแค่ไหนที่ทำให้ผู้คนมองทุกอย่างผ่านแว่น PR
    • น่าลองดู jj, sapling, mercurial

  • บล็อกโพสต์ดี และดีใจที่เห็นคนคิดจริงจังว่า code review ควรทำงานอย่างไร
    ผมเคยใช้ระบบ code review สี่แบบที่มีข้อดีข้อเสียต่างกันมามาก: Critique (ภายใน Google), Gerrit (ใช้ที่ Google แต่เหมือนกับภายนอก), GitHub และ CodeApprove ที่ผมสร้างเอง
    Critique ดีที่สุดอย่างท่วมท้น แต่ทำได้เพราะมันเข้ากับ monorepo ของ Google, ระบบจัดการเวอร์ชันของตัวเอง และเครื่องมือ lint/test แบบ custom อย่างสมบูรณ์ CodeApprove ถูกออกแบบมาเพื่อพยายามนำข้อดีเหล่านั้นมาไว้บน GitHub ให้มากที่สุด แต่ในทางปฏิบัติการเข้าใกล้ระดับนั้นเป็นเรื่องยาก
    Gerrit ดีเป็นอันดับสองในแง่ประสบการณ์ของ reviewer แต่ในฐานะ author ผมไม่เคยชอบเลย ดูเหมือนจะมีวิธีทำผิดมากกว่าวิธีทำให้ถูก และ UI ก็ไม่ได้สวยงามอะไรนัก
    GitHub เป็นมิตรกับ author มาก และทำงานตามวิธีที่เราคิด คือเขียนโค้ด รับ feedback แล้วเขียนโค้ดเพิ่ม ถ้า squash and merge ตอนท้ายของ PR ก็ไม่มีปัญหาเรื่องประวัติอย่างที่ผู้เขียนพูดถึง
    แต่ไม่ค่อยเป็นมิตรกับ reviewer หรือทีมเท่าไร incremental diff ไม่ได้ถูกเน้น diff กับบทสนทนาอยู่คนละแท็บ force push กับ rebase ทำลายประวัติ คอมเมนต์หายไปเป็น “outdated” คอมเมนต์ไฟล์ที่อยู่นอกหน้าต่าง diff ไม่ได้ ไฟล์ใหญ่ถูกซ่อนไว้เป็นค่าเริ่มต้น และยังมีปัญหาอีกมาก เห็นได้ชัดว่า GitHub ไม่ค่อยใส่ใจเรื่องนี้ และบางทีมันอาจรู้อะไรบางอย่างที่ผมไม่รู้ก็ได้
    ท้ายที่สุด สิ่งที่น่าหงุดหงิดที่สุดคือหลายทีมยอมรับเครื่องมือ code review ที่ฝังมากับแพลตฟอร์มจัดการเวอร์ชันไปเฉย ๆ เหมือนใช้ IDE ที่ติดมากับโน้ตบุ๊กโดยไม่เปลี่ยน ตอนนี้มีตัวเลือกที่ดีกว่ามากแล้ว นอกจาก CodeApprove ตัวที่ผมชอบคือ GitContext, Reviewable, Graphite และยังยกตัวเลือกดี ๆ อื่นได้อีกอย่างน้อยครึ่งโหล อย่ายอมรับค่าเริ่มต้น

    • ปัญหาคือ GitHub ไม่เป็นมิตรกับ reviewer หรือทีม
      reviewer และ maintainer หายากกว่าผู้มีส่วนร่วมมาก ผมคิดว่า workflow และ UI ควรถูกปรับให้เหมาะกับ throughput ของการรีวิว
      ผมไม่เคยใช้เครื่องมืออีกสามตัวที่กล่าวถึง ดังนั้นข้ออ้างของผมเป็นการพูดในภาพรวม