Perbandingan Unified Diff dan Split Diff

 (matklad.github.io)
3 poin oleh GN⁺ 2023-10-25 | 1 komentar | Bagikan ke WhatsApp
  • Untuk perubahan kecil, baik unified diff maupun split diff sama-sama cukup memadai untuk ditinjau, tetapi pada perubahan yang besar dan kompleks, review yang hanya membaca diff akan menemui batasnya
  • Perubahan besar perlu ditelusuri dan diverifikasi berdasarkan keadaan keseluruhan codebase pada titik waktu tertentu, bukan berdasarkan riwayat perubahan
  • Tampilan review yang ideal harus menampilkan kode saat ini di disk dan unified diff untuk area tersebut secara berdampingan, agar konteks kode dan isi perubahan bisa diikuti sekaligus
  • Karena dukungan dari alat yang ada masih kurang, digunakan workflow berteknologi rendah dengan checkout PR secara lokal, lalu menghapus commit dan hanya menyisakan perubahannya
  • Dengan cara ini, penelusuran file yang berubah dan penandaan hunk yang sudah direview bisa dilakukan, tetapi posisi saat ini di editor dan diff tidak tersinkron otomatis sehingga masih perlu operasi manual

Mereview keadaan kode saat ini, bukan sekadar diff

  • split diff dan unified diff sama-sama bekerja baik untuk perubahan kecil dan sederhana
  • Saat perubahan menjadi besar dan kompleks, hanya membaca diff saja tidak lagi cukup
  • Untuk perubahan besar, kita perlu benar-benar meninjau codebase pada titik waktu tertentu
    • Tetap memberi perhatian pada area yang baru berubah, tetapi pada dasarnya harus melihatnya seperti code review biasa
    • Perlu menjalankan test
    • Perlu menggunakan fitur navigasi editor seperti goto definition
    • Perlu menerapkan perubahan lokal untuk memeriksa apakah ada cara penulisan lain yang mungkin lebih baik
    • Perlu mencari bagian yang seharusnya ikut berubah dalam konteks yang lebih luas
    • Harus bisa menemukan hal-hal yang janggal di codebase saat ini, terlepas dari riwayat perubahannya

Tampilan review yang diinginkan dan cara memutar lewat lokal

  • Tampilan diff yang ideal adalah bentuk yang menampilkan keadaan kode saat ini di kiri, dan unified diff untuk area kode yang sedang terlihat di kanan
    • Kode di kiri harus sama dengan keadaan saat ini yang ada di disk
    • Perubahan ditandai secara halus di margin
    • Diff di kanan harus sesuai dengan area kode yang sedang terlihat di sisi kiri
  • Dukungan alat untuk format review seperti ini saat ini masih kurang
  • Workflow yang benar-benar dipakai adalah checkout PR secara lokal dengan gpr script
    • Pada langkah terakhir, commit PR dihapus dan hanya perubahan yang disisakan
    • Dalam keadaan ini, workflow untuk staging dan commit dimanfaatkan untuk code review
  • Dengan edamagit, kita bisa melihat daftar file yang berubah dan berpindah ke perubahan berikutnya/sebelumnya di editor
    • staging area juga bisa dipakai untuk menandai hunk yang sudah direview
  • Ketidaknyamanan yang tersisa adalah tidak adanya sinkronisasi otomatis antara magit status buffer dan file yang sedang terbuka di editor
    • Untuk melihat file saat ini dan diff secara berdampingan, kita harus membuka diff secara manual lalu menggulir sendiri sampai ke posisi yang sedang dilihat
  • Untuk melakukan review yang sedekat ini dengan kode, saat ini kita masih harus membuat alat sementara sendiri
  • Tujuan utama code review tidak selalu harus semata-mata memeriksa kode; sebagai bacaan terkait, lihat juga Two Kinds of Code Review

Bacaan terkait

1 komentar

 
GN⁺ 2023-10-25
Komentar Hacker News

  • Pernyataan bahwa pada perubahan besar orang tidak ingin melakukan review diff, melainkan ingin meninjau codebase pada titik waktu tertentu dengan benar dan memberi perhatian pada area yang baru berubah, menurut saya pribadi tampaknya tidak cocok dengan code review pada umumnya
    Tentu berbeda menurut tim dan tiketnya, tetapi jika reviewer bukan peran seperti principal engineer yang terutama bertanggung jawab atas seluruh kode, biasanya code review lebih dekat ke pemeriksaan kewajaran sederhana oleh sepasang mata kedua
    Jika commit dalam PR tertata rapi atau sudah di-squash, perubahan inkremental juga relatif mudah ditinjau secara terpisah
    Pada akhirnya, keluhannya tampak lebih seperti perbedaan jenis review. Ada review pemeriksaan kewajaran yang umum dan review pre-merge arsitektur/fungsi yang mendalam, tetapi code review di alat web kebanyakan tampaknya lebih dekat ke yang pertama

    • Menganggap code review sekadar pemeriksaan kewajaran adalah sudut pandang yang terlalu miskin. Code review adalah mekanisme inti untuk mengurangi kepemilikan kode pribadi, menyebarkan konvensi, dan melakukan transfer teknologi
      Code review yang baik dimulai dari PR yang baik. PR harus menjelaskan tujuan dan bagaimana tujuan itu dicapai
      Item pertama dalam review yang baik adalah memastikan apakah kode mencapai tujuan yang dijelaskan; kedua, apakah ada test yang memverifikasi fungsi yang diklaim; ketiga, apakah mengikuti konvensi arsitektur sistem yang ada; keempat, apakah style sesuai ekspektasi; dan terakhir, apakah ada saran untuk membuat penggunaan API lebih baik
      Code review yang hanya melakukan pemeriksaan kewajaran tidak banyak berguna karena itu bisa dilakukan CI. Code review adalah proses manusia, jadi harus memaksimalkan hal-hal yang hanya bisa dilakukan manusia, dan menyerahkan sisanya ke mesin
      Dalam item-item di atas tersirat pertanyaan, “Apakah perubahan ini akan menjadi contoh yang baik untuk kontribusi di masa depan?”
    • Menurut saya code review yang dangkal hanya menghasilkan hasil yang dangkal. Inilah alasan banyak bug tetap ter-deploy meskipun code review sudah umum
      Saya selalu melakukan code review dari sudut pandang tester. Saya membangun dan menjalankan kode untuk melihat apakah bekerja sesuai maksud, mencari kelemahan, dan mencoba memecahkannya
      Diff bukan akhir, melainkan awal. Itu hanya petunjuk yang memberi tahu bagian kode mana yang perlu ditusuk-tusuk, dan pada akhirnya pembuktiannya berasal dari perilaku nyata
      Orang bisa berkata, “Itu pekerjaan tester, bukan pekerjaan engineer,” tetapi siapa yang bisa melakukan testing lebih baik daripada orang yang memahami kode dan tahu harus melihat ke mana?
      Saya tahu sikap seperti ini minoritas. Banyak engineer tampaknya memiliki penolakan yang aneh terhadap menjalankan kode sendiri, bahkan terhadap penggunaan debugger. Mereka membangun infrastruktur raksasa untuk mengotomatiskan semuanya, tetapi otomatisasi pada akhirnya hanyalah kode yang juga bisa memiliki bug. Siapa yang menguji test-nya, Quis custodiet ipsos custodes?
    • Saya setuju dengan hal-hal yang diharapkan dari code review, tetapi agar berjalan dengan benar, cara kerja tertentu harus lebih dulu tertanam
      Code review adalah garis pertahanan terakhir selain CI, tetapi di tim-tim tempat saya bekerja, arah besar yang dibawa PR sudah didiskusikan dengan beberapa orang sebelum titik itu. Caranya bisa pairing, penjelasan mendalam, coffee chat, atau apa pun yang sesuai dengan kompleksitasnya
      Dengan begitu, PR menjadi tempat untuk meninjau taktik yang mengimplementasikan strategi tersebut, bukan strategi itu sendiri seperti memisahkan modul baru, memperkenalkan dependensi besar, atau mengerjakan ulang permukaan API
      Jika hanya memeriksa kewajaran sebagian kode tanpa komunikasi sebelumnya, ada risiko kepemilikan kode terpecah-pecah. Keadaannya menjadi seperti, “Modul itu buat apa?” “Tidak tahu, tanya Bob”
      Terlepas dari itu, apa pun mode review-nya, saya suka ide tampilan diff yang diusulkan penulis
    • Ada juga hal-hal bagus yang dikatakan, tetapi setelah berulang kali mengalaminya, review diff sering kali benar-benar tidak bisa dipahami sehingga menjadi agak tidak bermakna
      Sulit melakukan review apa pun dengan gumpalan baris merah dan hijau yang bercampur. Itu bukan tampilan kode yang berguna, dan setidaknya tidak layak sebagai tampilan default
      Tetap saja, karena itu pekerjaan, saya memaksakan diri melakukannya, tetapi saya 100% setuju dengan penulis bahwa itu bukan tampilan yang terlalu berguna
    • Budaya perusahaan teknologi modern sudah terlalu jauh dari proses yang lebih formal untuk mengelola pengembangan
      Karena tidak ada proses dasar untuk menangani defect, fitur, dan sebagainya, setiap engineer harus menciptakan sendiri setiap kali bagaimana sebuah fitur akan dirilis melalui kolaborasi
      Kebanyakan akhirnya hampir tidak melakukan apa-apa, dan code review menjadi satu-satunya kolaborasi yang berlangsung pada tingkat detail bermakna tentang topik itu. Akibatnya, semua intervensi yang mungkin coba dijejalkan ke satu momen

  • Sebagai pilihan ketiga, atau keempat, difftastic juga layak disebut. Alih-alih diff per baris, ia menggunakan diff “struktural” sehingga menyoroti perubahan dengan lebih halus
    https://github.com/Wilfred/difftastic

    • Sebagai pilihan keempat, atau kelima, ada juga patdiff: https://opensource.janestreet.com/patdiff/
      Seingat saya, kadang sekitar 35% diff jadi lebih mudah dibaca, biasanya 60% tidak ada bedanya, dan sesekali 5% justru jadi lebih sulit dibaca
      Saya memakainya beberapa tahun lalu, jadi tidak terlalu ingat masalah spesifiknya. Satu-satunya alasan saya berhenti adalah karena mulai memakai magit untuk Git diff
    • Ini pendekatan yang sangat bagus. Hanya saja sayang belum terintegrasi secara luas. Sejauh yang saya tahu, output JSON terstruktur yang layak untuk digunakan alat lain juga baru muncul belakangan ini
    • ediff di Emacs juga melakukan ini. Fitur untuk menyoroti kata yang berbeda adalah Refine
    • git diff --word-diff juga punya fitur serupa dan cukup bagus

  • Saat meninjau dengan Vim pun rasanya mirip
    Ada sedikit skrip untuk membuka PR, lalu untuk file yang berubah kira-kira menjalankan sesuatu seperti vimdiff <(git show baseref:file) file. Tab di Vim pada dasarnya adalah view, jadi bagus karena masing-masing buffer bisa dibuka bersamaan dalam keadaan berbeda
    Di view default, scroll lock bekerja seperti yang diharapkan, dan jika perlu bisa dihindari di tab terpisah
    [c dan ]c berpindah antar hunk berdasarkan kumpulan perubahan yang ada di PR, bukan direktori kerja
    Dengan dp dan dg, hunk bisa di-push/pull ke buffer diff read-only untuk ditandai “selesai”, lalu disembunyikan dari perubahan yang disorot di buffer live
    Perubahan yang dibuat di buffer live bisa langsung di-commit atau di-push sebagai komentar
    Fitur editor umum seperti go to definition, integrasi build, dan dokumentasi pop-up juga tetap berfungsi pada keadaan tree saat ini
    Cara seperti ini memungkinkan melihat perubahan PR terhadap branch acuan sambil menjaga direktori kerja tetap bersih. Kelihatannya jauh lebih baik daripada solusi matklad yang melihat perubahan dengan direktori kerja dalam keadaan kotor
    Di lingkungan tempat saya bekerja, perubahan sering ditambahkan sebagai commit --fixup, dan tool menggabungkannya kembali saat merge dengan memanggil git-interpret-trailers agar penulis commit fixup diatribusikan ke commit asli, jadi ini sangat berguna. Tool juga mengambil komentar teks dari PR lalu, jika sesuai, menambahkan trailer Reviewed-by, atau jika setara +1 menambahkan trailer Acked-by

    • Saya penasaran apa tepatnya maksud “di-push sebagai komentar” dan “mengambil komentar teks”. Apakah ini semacam logika kustom khusus internal perusahaan?

  • Pada codebase yang sangat besar dan kompleks dengan banyak engineer, ini adalah masalah besar
    Alasan code review sulit adalah karena diff selalu terlihat masuk akal, tes selalu lolos, dan hal-hal dasar selalu sudah dicek
    Namun meski perubahan tampak masuk akal, sering kali kenyataannya tetap salah
    Satu perubahan buruk yang tampak masuk akal bisa membuat seluruh arsitektur melenceng
    Seperti biasa, secara ketat ini bukan masalah tool, melainkan lebih dekat ke masalah budaya dan berbagi pengetahuan. Tool yang lebih baik saja tidak akan menyelesaikannya

    • Sulit menerima dua kalimat “tes selalu lolos” dan “meski perubahan tampak masuk akal, sering kali salah” secara bersamaan
      Saat code review, jarang sekali kita mencari bug itu sendiri. Yang dilihat biasanya apakah ada tes yang bisa menangkap bug semacam itu

  • Sepertinya ada sesuatu yang terlewat. Penulis bilang split diff tidak cocok untuknya, tetapi tidak menjelaskan alasannya
    Diff ideal hampir terlihat seperti split diff dengan konteks duplikat dihilangkan. Bentuknya konteks hanya ada di kiri, bukan digandakan di kedua sisi
    Saya penasaran manfaat apa yang muncul jika konteks duplikat dihapus dari panel kanan

    • Penulis menulis bahwa ia perlu menjalankan tes, menggunakan go to definition dan fitur navigasi editor lain, menerapkan perubahan lokal untuk melihat apakah bisa ditulis dengan cara lain, melihat konteks yang lebih luas untuk menemukan bagian yang seharusnya ikut berubah, serta menyadari bagian yang aneh di codebase saat ini terlepas dari jalur perubahan historisnya
      Editor/IDE yang pernah saya pakai biasanya dukungannya untuk hal-hal seperti ini di dalam diff masih sangat dasar. Go to definition pun hanya bekerja di file yang sama, autocomplete sering tidak berfungsi baik untuk item yang baru ditambahkan, dan sering kali tidak ada fitur refactoring
    • Sisi kiri menampilkan kode saat ini tanpa modifikasi. Jika satu sisi menampilkan konteks penuh dengan fitur navigasi kode, dan sisi lainnya menampilkan perubahan, kita bisa melihat perubahan apa yang diterapkan pada konteks tertentu
      Sejauh pemahaman saya, penulis ingin membalik diff. Alih-alih melihat perubahan itu sendiri, ia ingin melihat kode dan memeriksa apakah ada perubahan terkait di sana
      Tulisannya kurang detail, tetapi tampaknya ia ingin menavigasi ke implementasi atau lokasi pemanggilan dan memeriksa apakah ada perubahan yang tepat
    • Saya bukan penulisnya, tetapi saya bisa memahami
      Highlight diff bisa lebih mengganggu daripada yang dikira, dan jika dimatikan, kadang keterbacaan jelas membaik. Saya juga kadang mematikannya di diff viewer untuk melihat kode dengan mata segar, tetapi tidak ada opsi untuk sekaligus menampilkan unified diff di split pane

  • Jika menekan . di GitHub, Anda masuk ke IDE penuh di dalam browser
    Ini sangat berguna saat review. Alih-alih hanya melihat potongan, kita bisa melihat perubahan dalam konteks file penuh, sehingga jauh lebih mudah menangkap masalah desain yang subtil

    • Ini benar-benar bagus. Saya hanya pernah masuk ke IDE ini saat mengedit file langsung di GitHub, tidak tahu ada shortcut langsung dari commit diff
      Sebagai catatan, shortcut ini juga berfungsi di GitLab

  • Saya merindukan beberapa fitur p4merge. Misalnya, ada fitur yang menampilkan bagian mana dari satu file yang berkorespondensi dengan bagian mana dari file lain di ruang antara split diff, sekaligus menunjukkan dampak diff di kedua sisi: https://www.perforce.com/manuals/p4merge/Content/P4Merge/dif...
    Saya juga menyukai fitur 3-way merge: https://www.perforce.com/manuals/p4merge/Content/P4Merge/dif...
    Fitur-fitur seperti ini belum masuk ke tool diff berbasis web yang banyak dipakai. Menurut saya, 3-way diff adalah cara yang baik untuk menunjukkan hasil merge yang sulit

  • Jika memakai GitHub, Anda bisa menekan . di halaman pull request github.com atau mengganti domain menjadi github.dev
    Maka VSCode akan terbuka di browser, dan pull request ditampilkan sebagai diff view
    Kelebihannya adalah Anda bisa melihat file lengkap, dan karena algoritma diff-nya berbeda dari github.com, kadang lebih mudah dibaca pada diff yang kompleks

    • Kekurangannya adalah lambat dan banyak bug
      View side-by-side diff yang menampilkan komentar PR sebagai daftar di luar baris bisa sangat berguna, tetapi sayang harus menjalankan seluruh instance VSCode di browser
      Sebenarnya fitur itu begitu berguna sampai GitHub dulu pernah punya split view seperti ini, dan tidak perlu mendorong orang ke tumpukan buzzword seperti lingkungan pengembangan AI/ML/crypto/blockchain/cloud-enabled
      Idenya bagus, tetapi eksekusinya buruk. Rasanya itu juga merangkum GitHub secara umum akhir-akhir ini

  • Meld cukup cocok untuk penggunaan seperti ini
    https://meldmerge.org/

    • https://kdiff3.sourceforge.net/ juga punya banyak tampilan yang bagus, jadi layak disebut bersama
    • Terlihat cukup mirip dengan FileMerge yang disertakan dalam alat pengembang Apple

  • Alat favorit saya adalah diff2html-cli: https://diff2html.xyz/
    https://www.npmjs.com/package/diff2html-cli
    diff bisa dilihat sebagai HTML, dan mendukung bentuk side-by-side maupun unified