Tujuan utama code review adalah menemukan kode yang sulit dipelihara

• Code review lebih merupakan proses untuk sejak awal mengungkap kode yang sulit dipelihara di kemudian hari, daripada prosedur untuk menemukan bug atau menjamin integritas
• Jika reviewer membaca kode tetapi sulit memahaminya, kemungkinan besar maintainer di masa depan akan mengalami masalah yang sama
• Perbaikan sebaiknya dilakukan sekarang, saat penulis aslinya masih mengingat konteksnya, dan ekspektasi bahwa bug dapat ditemukan secara andal hanya lewat pemeriksaan kode tidak realistis
• Bagi reviewer, tugas “coba pahami, lalu tandai bagian yang tidak dipahami” lebih dapat dijalankan daripada “temukan bug”
• Review yang baik bukan soal membuktikan semuanya secara sempurna, melainkan dimulai dari meninggalkan catatan dan meminta perbaikan pada titik-titik yang tidak dipahami

---

Mengubah fokus code review

• Tujuan inti code review bukanlah reviewer menemukan bug, dan juga bukan menjamin bahwa kode tidak memiliki bug
• Ekspektasi bahwa bug secara umum dapat ditemukan hanya dengan menelusuri kode secara sekilas lemah dalam praktik
• Karena itu, pusat perhatian review bukan “apakah ini kode yang benar”, melainkan “apakah orang lain dapat membacanya dan memperbaikinya nanti”

Kode yang sulit dipahami adalah sinyal risiko pemeliharaan

• Reviewer membaca kode untuk mencoba memahami apa yang dilakukan kode dan bagaimana cara kerjanya
• Bagian yang tidak dipahami menjadi sinyal risiko bahwa maintainer di masa depan mungkin akan tersendat
• Kode seperti ini sebaiknya segera diperbaiki saat penulis aslinya masih mengingat konteksnya

Tugas review yang lebih dapat dijalankan daripada mencari bug

• Permintaan “coba temukan bug dalam kode ini” adalah pekerjaan yang sulit dinilai keberhasilannya
• Walaupun beberapa bug ditemukan, mungkin masih ada bug lain yang tersembunyi dan terlewat, sehingga dari sudut pandang reviewer yang sering menjadi jelas hanyalah kegagalan
• Sebaliknya, permintaan “coba pahami kode ini, dan tunjukkan jika ada bagian yang tidak dapat dipahami” memiliki kriteria yang lebih jelas
  • Tidak perlu memahami semuanya secara sempurna
  • Cukup mencatat bagian yang tidak dipahami
  • Jika sudah mencoba memahami keseluruhan dan meninggalkan catatan pada titik yang membuat tersendat, berarti peran review sudah dijalankan

Kriteria review dalam praktik

• Kode yang tidak dipahami reviewer dengan sendirinya dapat menjadi target perbaikan
• Komentar review tidak hanya berperan sebagai laporan bug, tetapi juga mengungkap kurangnya penjelasan, masalah struktur, dan alur yang sulit dibaca
• Dengan kriteria ini, yang lebih penting daripada membuktikan kebenaran kode adalah membuatnya berada dalam kondisi yang dapat dibaca dan ditangani oleh anggota tim di kemudian hari

Penemuan bug adalah efek samping

• Ini bukan berarti code review sama sekali tidak dapat menemukan bug
• Bug bisa ditemukan selama review, tetapi sulit mengharapkannya sebagai metode untuk menemukan semua atau sebagian besar bug
• Syarat sukses yang lebih realistis adalah memeriksa keterpahaman, lalu segera memperbaiki bagian yang sulit dipelihara bersama penulis aslinya

[GN⁺]

Opini Hacker News

• Code review tidak punya satu tujuan tunggal. Menemukan kode yang sulit dipelihara itu penting, tetapi mungkin bukan satu-satunya tujuan, dan bisa jadi juga bukan yang paling penting
  Ini menjadi pengaman yang mempersulit penggabungan kode berbahaya ketika developer atau AI mulai bergerak ke arah yang aneh, dan orang yang tidak terlalu dekat dengan masalah sering kali bisa melihat cara yang lebih baik atau masalah yang terlewat
  Seseorang yang lebih paham bagian lain dari sistem juga bisa menangkap masalah interaksi, setidaknya ada satu orang yang jadi akrab dengan kode tersebut, dan ini menjadi kesempatan belajar bagi penulis maupun reviewer
  Ini особенно penting ketika ada perbedaan pengalaman; saat membimbing karyawan baru, saya menambahkan mereka sebagai reviewer di semua PR saya agar mereka bisa melihat cara saya bekerja, dan saya juga mereview PR mereka untuk memberi arahan. Kadang saya juga belajar dari mereka
  Menangkap bug memang bagian dari itu, tetapi seharusnya bukan mekanisme utamanya, dan ini sangat penting terutama untuk bug keamanan dan performa yang sulit ditangkap dengan automated test

  • Menambahkan satu alasan lama lagi: orang menulis kode secara berbeda ketika mereka tahu kodenya akan direview dan dari kode itu akan terbentuk kesan jangka panjang tentang kemampuan dan kecocokan mereka dengan organisasi
  • Mirip dengan poin 3, orang yang lebih akrab dengan subdomain tersebut bisa menangkap bagian yang tidak cocok dengan kode yang ada atau bagian yang berpotensi menimbulkan masalah
  • Saya membaca “tujuan tunggal” sebagai memahami kode dan mempertanyakan bagian yang tidak dipahami. Jika maksudnya adalah bahwa setelah memahaminya kita bisa menunjukkan bagian yang salah, bodoh, atau tidak aman, itu masuk akal
    Ini terutama berlaku pada modularisasi dan dekomposisi; setelah memahami seluruh PR besar, kita membentuk model mental sehingga mulai terlihat apakah ini akan bisa dipelihara, akan menjadi mimpi buruk total suatu hari nanti, atau ada di antara keduanya

• Mungkin bagian terpenting dari code review adalah transfer pengetahuan
  Tim kecil kami, kecuali jika benar-benar mendesak, meminta seluruh tim memberi persetujuan pada PR sebelum merge, dan berkat itu semua anggota tim kira-kira tahu kondisi codebase saat ini. Ini membantu menghindari kejutan seperti “seluruh sistem yang saya andalkan ternyata sudah hilang”, sesuatu yang pernah saya alami di perusahaan yang lebih tersilo
  Ini juga menjadi tempat untuk bertanya soal cara kerja sesuatu dan memperdalam pemahaman. Dalam tim yang berjalan baik, semua developer seharusnya memahami keseluruhan sistem sampai tingkat tertentu, termasuk bagian yang tidak mereka sentuh langsung
  Fungsi penting lainnya adalah pemeriksaan pengetahuan organisasi. Saya baru-baru ini sedikit mengubah sebuah tabel, lalu seorang rekan memberi tahu bahwa ada microservice yang tidak saya pertimbangkan yang menulis ke tabel itu dan akan rusak karenanya. Saya bahkan tidak tahu microservice itu ada, apalagi bahwa ia mengakses tabel tersebut, tetapi pemeriksaan itu mencegah masalah yang lebih besar dan situasi pembersihan data

  • “Seluruh tim kecil memberi persetujuan pada PR sebelum merge” terdengar bagus untuk codebase kecil yang bergerak lambat, tetapi jika dipaksakan pada tim besar atau codebase yang bergerak cepat, ini mudah berubah menjadi ritual formalitas berupa melihat sekilas lalu menekan tombol approve
    Pada akhirnya semua orang sibuk dengan pekerjaannya sendiri dan tidak ingin menjadi orang yang menghambat PR penting, jadi mereka hanya menekan approve, sementara kenyataannya tidak ada yang benar-benar mereview kode
  • Saya penasaran seberapa besar ukuran timnya. Mungkin cara seperti itu tidak akan skala jika jumlah engineer lebih dari lima
    Untuk masalah seperti “seluruh sistem yang saya andalkan ternyata sudah hilang”, saya rasa automated test sangat penting agar masalah semacam itu tetap bisa tertangkap meski orang yang bergantung pada sistem itu tidak sedang ada di tempat
    Saya juga sangat mendukung kepemilikan bersama atas semuanya. Wajar jika orang akhirnya secara de facto memiliki bagian tertentu dari codebase, terutama komponen yang mereka buat sendiri, tetapi itu mengarah pada silo dan bus factor yang rendah. Tidak seharusnya ada struktur di mana satu orang memiliki satu sistem, lalu sistem itu bergantung pada satu komponen lain
  • Jika rekan itu tidak ada di review, saya penasaran apa yang akan mencegah perubahan yang merusak itu lolos ke production. Jika code review penting, di mana posisi test
  • Kadang saya tetap membuat PR dan menandai developer lain, bahkan untuk kode yang memang akan segera di-merge. Tujuannya adalah menyediakan jalur yang mudah untuk melihat apa yang di-merge dan kenapa, agar orang tidak kehilangan konteks tentang apa yang ada di dalam codebase
  • Untuk perubahan yang tidak sepele atau pekerjaan perapihan, pandangan kedua selalu bagus. Tetapi pendekatan “semua orang membaca semuanya” tidak bisa skala ke N yang besar
    Kalau bahan bacaan sudah terlalu banyak, tidak ada yang bisa mengikuti semuanya, itulah sebabnya kita mendelegasikan, membuat dokumentasi, dan mengadakan sesi gambaran umum

• Saya selalu berpikir cara terbaik memandang code review adalah sebagai gerbang ketika kode yang tadinya dimiliki penulis berpindah menjadi milik tim atau proyek
  Kode yang saya review bukan kode Anda, melainkan kode yang sebentar lagi akan menjadi kode kita. Tentu saja maintainability adalah faktor besar di dalamnya

  • Benar-benar lingkungan yang membuat iri dan mewah
    Tim kami mulai menggunakan AI, jadi saya beralih ke pendekatan sederhana. Saya tidak meninggalkan komentar, hanya membuat keputusan persetujuan biner: “ini sudah tingkat gila atau masih bisa lolos”
    Saya sedang menghemat waktu dan kewarasan saya

• Melakukan ini hanya akan membuat reviewer dan penulis jadi lebih malas
  Tujuan code review itu berlapis. Apakah kode sulit dipelihara, apakah mungkin ada bug, apakah bisa dibuat lebih sederhana dan rapi, apakah sesuai dengan style kode proyek, apakah membantu orang lain memahami kodenya, apakah membantu onboarding anggota tim junior, apakah menjadi sanity check untuk keputusan desain — semuanya termasuk
  Tulisan ringan seperti ini pada dasarnya lebih dekat ke upaya reviewer kode yang malas untuk membenarkan diri sendiri

  • Ada checklist tersendiri untuk hal-hal yang perlu dilihat saat review
    Harus dilihat apakah secara fungsional tujuan tercapai sesuai issue atau penjelasan PR, apakah ada kode tak perlu seperti output debug yang tertinggal atau API key privat, dan apakah ada cacat yang jelas seperti memory leak, edge case yang tidak tertangani, celah keamanan, atau pemanggilan API yang usang
    Perlu juga dilihat apakah kode bisa dibuat lebih mudah dipahami, apakah abstraksi perlu ditambah atau dikurangi, apakah nama variabel dan metode bisa dibuat lebih baik, dan apakah lebih baik memakai gaya fungsional lebih banyak atau lebih sedikit
    Harus dipastikan juga apakah konsisten dengan codebase atau style guide, apakah ada peningkatan performa yang jelas seperti memakai hash set alih-alih list atau menerapkan lazy evaluation, dan apakah pengujiannya sudah memadai
    Saya juga tidak sepenuhnya setuju dengan klaim bahwa kode yang tidak bisa dipahami tidak boleh lolos. Ada kode yang memang sangat sulit dipahami, dan tujuannya adalah membuatnya benar secara fungsional sekaligus semudah mungkin untuk dipahami
  • Industri sudah lama berusaha keras beralih dari “menyalahkan penulis” ke “menyalahkan proses dan tim”
    Tapi tulisan ini nyaris cuma umpan, dan mirip dengan mengatakan, “orang mengira makan malam itu soal makan makanan, padahal sebenarnya bukan makan, melainkan terhubung dengan keluarga dan teman.” Ini jenis logika reduksionis yang longgar dan memang laris di HN
  • Sekarang, karena AI membuat penulisan dan deployment kode jadi lebih cepat, penekanannya harus bergeser ke review. Kita harus memastikan apakah kode benar-benar berjalan dengan semestinya, apakah semua asumsi kita benar, dan apakah tidak ada efek samping yang tidak diinginkan
    Saya merasa review dan debugging jauh lebih memakan waktu dibanding penulisan dan produksi kode, dan sekadar “berdoa semoga jalan” tidak pernah berakhir dengan baik

• Pernyataan “dengan melihat kode biasanya kita tidak bisa menemukan bug” sama sekali tidak benar. Di setiap tingkat abstraksi ada banyak hal yang bisa ditemukan, dan itulah yang disebut code smell
  File descriptor yang tidak ditutup, coroutine yang tidak di-await, blok try/catch besar yang mengembalikan suatu nilai tanpa mencatat error, type casting yang salah, semuanya termasuk
  Sebagai prinsip umum, type checker, compiler, dan runtime jangan dipandang hanya sebagai tahap yang harus dilewati. Kita harus bekerja bersama tahap-tahap ini, mengakui bahwa semuanya adalah alat yang bernilai, dan tidak bekerja melawan mereka

  • Saya tidak paham maksudnya. Saya pernah menemukan bug hanya lewat code review tanpa menjalankannya, dan sebaliknya orang lain juga pernah melakukan itu pada kode saya, dan saya juga pernah melihat hal seperti itu dalam review orang lain
    Mungkin “biasanya” bisa didefinisikan sedemikian rupa agar secara teknis benar, tapi kalau begitu jadi tidak banyak artinya

• Kalau ingin membuat klaim luas tentang code review, penting untuk mendefinisikan jenis code review yang dimaksud
  Saat ini review PR asinkron ala GitHub dengan komentar inline memang standar, tapi review bukan hanya itu. Dulu ada juga review tatap muka dengan proses yang lebih mirip sidang tesis atau presentasi konferensi
  Literatur yang mengatakan code review adalah praktik kualitas yang berguna — bahkan salah satu dari sedikit praktik kualitas yang benar-benar berguna — umumnya berasal dari proses review yang jauh lebih terstruktur dibanding sekarang
  Secara pribadi, sebelum era LLM, review PR ala GitHub terasa seperti sesuatu untuk membuat proses tampak memadai atau untuk mencentang kotak governance, dan di era LLM nilai guna dibanding biayanya terasa jauh lebih buruk sehingga saya rasa akan menghilang

  • Review sinkron masih sangat mungkin dilakukan sekarang. Salah satu manajer awal saya mengajarkan bahwa kalau code review “standar” bolak-balik lebih dari sekali, hampir selalu lebih baik bertemu langsung, atau kalau salah satunya remote, membereskannya lewat Zoom lalu meninggalkan komentar yang merangkum kesepakatan
    Kalau mau memakai analogi teknis yang dipaksakan, komunikasi teks asinkron punya lebih banyak kehilangan dan throughput lebih rendah dibanding ucapan dalam hal jumlah informasi yang bisa berhasil dienkodekan. Jadi ketika perlu bertukar banyak informasi, membayar biaya sinkronisasi itu layak
  • Di salah satu pekerjaan pertama saya, paket perubahan dicetak untuk direview dan harus ditandatangani. Bahkan ada orang yang bertugas menyimpan versi finalnya di lemari arsip
    Itu lebih dekat ke rekayasa tradisional, dan semua orang harus memandang perangkat lunak sebagai sesuatu yang lebih permanen

• Memang benar bahwa memastikan maintainability adalah salah satu manfaat code review, tetapi mengatakan bahwa itu satu-satunya tujuan adalah klaim yang cukup berani
  Code review juga merupakan alat yang membantu tim memahami perubahan kode dan membagi tanggung jawab bersama atas keseluruhan codebase

• Code review bukan satu hal yang tunggal. Ada banyak alasan: berbagi pengetahuan, pencucian tanggung jawab, kualitas kode, kepatuhan regulasi, dan seperti biasa tujuan yang diemban bergantung pada konteks penggunaan

  • Mengatakan bahwa kebanyakan orang tidak memahami alasan adanya peer review terdengar cukup bodoh. Keyakinan bahwa hanya ada satu tujuan saja sendiri terlihat seperti tanda kurangnya pengalaman bekerja dengan tim atau orang lain yang berbeda

• Jika penulisnya seorang matematikawan, maka pernyataan “dengan melihat kode biasanya kita tidak bisa menemukan bug” mungkin bukan berarti menemukan bug itu sama sekali mustahil
  Maksudnya lebih dekat ke bahwa tidak mungkin menemukan semua bug, atau menemukan bug tertentu

  • Dari pengalaman saya di kuliah matematika universitas, matematikawan sering kali benar-benar buruk dalam berkomunikasi dengan manusia lain. Jadi itu menjelaskan mengapa mereka mengira apa yang mereka katakan berbeda dari cara hampir semua orang membacanya
  • Kalau maknanya itu, ya benar
    Menambahkan kaitan dengan poin maintainability, salah satu tujuan review juga adalah membuat kode sesederhana mungkin agar peluang debugging lewat review meningkat. Itu tidak mencegah bug dalam arti absolut, tetapi meningkatkan probabilitasnya
  • Tampaknya penulis yang seorang matematikawan tidak memahami makna kuantifier bahasa alami yang ia pakai. “Dengan melihat kode biasanya kita tidak bisa menemukan bug” berarti “dengan melihat kode biasanya kita tidak bisa menemukan bug apa pun”, bukan “kita tidak bisa menemukan semua bug”
    Tafsiran pertama masih terkait tetapi salah, sedangkan tafsiran kedua benar namun tidak relevan
    Mungkin yang ingin dikatakan penulis adalah “untuk bug tertentu, biasanya kita tidak bisa menemukannya hanya dengan melihat kode”, yakni “untuk setiap bug B, tidak selalu mungkin menemukan B”, tetapi ini pun meski benar tetap jauh dari pokok persoalan

• Pernah bekerja bersama orang yang sering menolak saran PR dan orang yang menerima saran.
  Orang yang menerima saran membuat saya berpikir bahwa mereka ingin berbagi rasa kepemilikan dengan saya sampai tingkat tertentu. Terasa bahwa kami sama-sama memelihara dan memahami kode, serta melihat ke arah yang sama.
  Sebaliknya, terhadap PR dari orang yang menolak saran PR, keinginan untuk berpartisipasi jadi berkurang. Kalau toh pada akhirnya akan ditolak, untuk apa meluangkan waktu mereview dengan teliti?

  • Tim kami umumnya menambahkan prefiks seperti thought, change, nit, fix, chat di awal komentar.
    Misalnya, thought berarti “nanti saat foo jadi lebih umum, mari refactor waktu itu”, change berarti “ini abstraksi yang bocor jadi saya ingin dimodelkan seperti bar”, nit berarti “namanya agak kurang intuitif jadi pertimbangkan Baz atau Boo”, fix berarti “unit test ini memverifikasi field yang salah”, dan chat berarti “ini keputusan besar yang akan menentukan bentuk solusi dalam kategori ini ke depan, jadi mari diskusikan dulu dengan tim”.
    Ada prefiks yang membuat PR tertahan sampai diperbaiki, dan ada juga yang berarti komentarnya boleh diikuti atau tidak. Ini memperjelas bagi penulis mana yang merupakan “hal yang harus mencapai pemahaman yang sama” dan mana yang sekadar “preferensi ungkapan” atau “pengamatan”.
    Namun, kalau Anda meninggalkan nit lalu pihak lain tidak setuju dan mengabaikannya, Anda tidak boleh tersinggung. Kalau Anda merasa kuat soal itu, berarti itu seharusnya bukan nit.
  • Jika memang dianggap penting, tinggalkan saran yang bersifat memblokir dan paksa terjadinya percakapan.