Antipola dalam Code Review

• Code review terdengar seperti ide yang bagus, bukan?
  • Dengan code review, dua developer melihat kode bersama-sama untuk menemukan masalah, dan ada kesempatan untuk berbagi pemahaman tentang proses perkembangan proyek
  • Reviewer dapat mempelajari trik yang berguna dengan melihat kode penulis secara detail, atau menemukan kesempatan untuk memberi tahu penulis trik yang berguna
• Namun, itu adalah cara yang digunakan oleh reviewer code review dari 'sisi terang'. Mereka menggunakan code review untuk memperbaiki kode dan meningkatkan kemampuan teknis kolektif para developer
• Code review juga bisa menjadi alat yang kuat untuk tujuan yang sama sekali berbeda. Jika reviewer beralih ke 'sisi gelap', mereka bisa memakai berbagai cara untuk menghambat atau menunda perbaikan kode
  • Mereka juga bisa mengejar tujuan pribadi lain, seperti mengganggu penulis patch atau membuatnya benar-benar frustrasi
• Jika Anda adalah reviewer yang baru-baru ini beralih ke 'sisi gelap', mungkin Anda belum memikirkan semua kemungkinannya
  • Karena itu, tulisan ini menyajikan daftar antipola untuk reviewer code review sisi gelap

[Antipola]

The Death of a Thousand Round Trips

• Saat membaca kode, begitu menemukan hal kecil, langsung tunjukkan lewat komentar review lalu segera berhenti membaca
• Developer dengan rajin memperbaiki hal kecil itu lalu mengirim ulang patch yang telah direvisi
• Reviewer mulai membaca versi itu, lalu menemukan hal kecil lain. Sebenarnya itu bisa disebutkan pada review pertama, tetapi tidak dilakukan. Hal kecil itu ditunjukkan lalu reviewer kembali berhenti membaca
• Ulangi ini sampai developer kehilangan harapan

The Ransom Note

• Patch ini tampaknya sangat penting bagi developer
• Namun bagi reviewer, itu tidak terlalu penting. Karena itu reviewer berada pada posisi berkuasa
• Reviewer dapat menyandera perubahan yang diinginkan developer sampai pekerjaan tambahan yang penting bagi reviewer selesai
  • Pekerjaan tambahan itu sebenarnya tidak perlu dimasukkan ke commit yang sama, tetapi itu penting bagi reviewer

The Double Team

• Satu patch, dua reviewer
• Setiap kali satu reviewer meminta perubahan, developer menurut dan mengubahnya. Lalu giliran reviewer lain untuk mengeluh
• Para reviewer bergantian mengajukan tuntutan yang saling bertentangan
  • Tetapi mereka selalu berkomentar kepada developer. Mereka menghindari berdebat langsung satu sama lain di thread review
• Tujuannya adalah melihat berapa kali reviewer bisa memantulkan developer ke sana kemari sampai ia menyerah

The Guessing Game

• Ada sebuah masalah, dan ada berbagai cara untuk menyelesaikannya
• Developer memilih satu solusi lalu mengirimkan patch
• Reviewer mengkritik solusi spesifik itu dengan alasan yang tidak ada hubungannya dengan apakah solusi itu benar-benar menyelesaikan masalah
  • Misalnya dengan alasan pelanggaran prinsip desain yang samar-samar atau ketidakcocokan dengan rencana pengembangan di masa depan
• Jika kritik dibuat cukup samar, developer tidak akan tahu bagaimana patch yang ada harus diubah agar kritik itu terjawab
  • Dalam posisi itu, cara terbaik bagi developer untuk menyelesaikan masalah awal adalah memilih solusi yang sama sekali berbeda dan mengimplementasikannya sebagai gantinya
• Lalu reviewer menolaknya lagi dengan cara yang sama-sama tidak membantu

The Priority Inversion

• Pada review kode pertama, tunjukkan hal-hal kecil dan sederhana terlebih dahulu
• Tunggu developer memperbaikinya, lalu angkat masalah besar
  • Ada masalah mendasar yang mengharuskan sebagian besar patch ditulis ulang sepenuhnya. Ini berarti banyak perbaikan kecil yang sudah dilakukan harus dibuang
  • Tidak ada yang lebih jelas menyampaikan pesan “pekerjaanmu tidak diinginkan, dan waktumu tidak berharga” selain menyuruh seseorang bekerja banyak lalu membuang hasilnya
• Ini saja bisa cukup untuk membuat developer menyerah

The Late-Breaking Design Review

• Selama berminggu-minggu atau berbulan-bulan, pekerjaan yang sangat kompleks telah berjalan lewat banyak patch terpisah
• Reviewer tidak setuju dengan desain keseluruhan pekerjaan itu, tetapi tidak ikut dalam diskusi awal atau berada di pihak yang kalah dalam perdebatan
• Lalu sekarang reviewer diminta meninjau satu bagian kecil tetapi penting di tengah pekerjaan itu, misalnya perbaikan mudah untuk bug yang menghambat banyak developer. Bagi reviewer, ini adalah kesempatan
• Reviewer menuntut pembenaran untuk keseluruhan desain dari semua pekerjaan yang telah dilakukan sampai saat ini
• Jika developer hanya menangani sebagian dari keseluruhan pekerjaan dan tidak tahu jawabannya, itu bukan masalah reviewer. Reviewer tidak akan menekan tombol “approve” sampai ia puas

The Catch-22

• Jika patch-nya satu dan besar, itu terlalu sulit dibaca. Minta agar dipecah menjadi bagian-bagian kecil yang self-contained
• Sebaliknya, jika ada banyak patch kecil, sebagian di antaranya tidak bermakna jika dilihat sendiri. Minta agar digabung lagi
• Jika tampaknya ada trade-off tertentu yang relevan pada kasus tertentu, gunakan itu untuk mencari alasan mengeluh
  • Misalnya, jika kode ditulis agar mudah dikenali maka performanya pasti buruk, dan jika dioptimalkan dengan baik maka pasti sulit dibaca dan sulit dirawat

The Flip Flop

• Ada jenis perubahan yang biasanya dilakukan orang pada bagian kode yang sama
• Reviewer sebelumnya sudah beberapa kali meninjau perubahan seperti ini
• Perubahan lain kembali masuk. Penulis telah melihat perubahan-perubahan sebelumnya dengan saksama, mengikuti pola yang ada dengan hati-hati, dan memilih reviewer yang tampak akrab dengan area ini
• Reviewer tiba-tiba mempermasalahkan satu aspek dari perubahan yang sebelumnya sama sekali tidak pernah dipermasalahkan. Mengikuti pola yang ada saja tidak cukup
• Apa yang terjadi jika penulis menunjukkan perubahan lama yang sama dan menyoroti ketidakkonsistenan reviewer?
  • Reviewer berkata, “Benar. Itu juga harus diubah”
  • Reviewer harus berhati-hati agar tidak sukarela mengubah semua contoh yang sudah ada. Kalau beruntung, developer akan menafsirkan ini sebagai instruksi untuk mengubah sendiri semua contoh yang ada sehingga reviewer bisa menghemat banyak usaha

Tapi serius ...

• Sampai di sini tulisan ini adalah lelucon. Ini juga bukan hendak menyatakan bahwa reviewer sengaja melakukan perilaku buruk seperti ini
  • Sebagian besar penjelasan di atas adalah dilebih-lebihkan dari hal yang benar-benar dilakukan reviewer, atau dari apa yang dirasakan oleh pengirim patch yang frustrasi
• Maksudnya adalah: jangan benar-benar melakukan hal-hal seperti ini!
  • Usahakan meminimalkan round trip, minta penulisan ulang besar lebih dulu (jika memang perlu) sebelum mengomentari hal-hal kecil, dan saat mengkritik patch berikan usulan yang konstruktif tentang versi seperti apa yang akan Anda terima
  • Jika Anda punya pendapat tentang keseluruhan codebase, lakukan diskusi umum dengan semua developer alih-alih mencari-cari kesalahan saat meninjau patch milik satu developer
  • Jika Anda melakukan hal seperti ini tanpa sengaja, sadari itu. Akui bahwa Anda tanpa sengaja telah membuat hidup developer lebih sulit, minta maaf, dan berusahalah menjadi lebih membantu
• Masalah mendasarnya adalah penyalahgunaan wewenang
  • Ketika satu developer menjadi reviewer kode untuk patch milik developer lain, ia memperoleh wewenang sementara. Reviewer memiliki kuasa untuk mencegah patch itu di-commit
  • Wewenang datang bersama tanggung jawab. Wewenang harus digunakan untuk tujuan yang semestinya, dan hanya sejauh yang diperlukan
  • Sebagian besar antipola ini (atau perilaku ringan yang disindir) adalah penyalahgunaan wewenang. Reviewer menggunakan wewenang sementara atas developer sebagai tuas untuk mengejar agenda pribadi yang tidak berhubungan dengan, atau bahkan bertentangan dengan, perbaikan patch
  • Agenda pribadi itu berbeda-beda menurut antipolanya: pekerjaan yang tidak relevan, preferensi gaya pribadi, kemalasan, resistansi terhadap perubahan, atau kebencian pribadi terhadap pengirim patch
• Jika pengirim patch pernah berperilaku buruk seperti ini saat menjadi reviewer di masa lalu, mungkin kebencian itu bisa dibenarkan. Itulah sebabnya wewenang reviewer kode harus digunakan dengan hati-hati
  • Jika para developer terjebak dalam lingkaran balas dendam satu sama lain, proyek perangkat lunak akan hancur

Code review gatekeeping

• Sampai di sini pembahasannya berfokus pada code review antarsesama
  • Reviewer kode dan pengirim patch adalah rekan sejawat; mereka tidak saling membawahi dan tidak memiliki keputusan akhir atas codebase
  • Karena itu, wewenang yang diperoleh dalam code review bersifat sementara
• Dalam situasi peer review, reviewer kode dan penulis pada dasarnya harus memiliki tujuan yang sama
  • Jika ada ketidaksepakatan serius tentang fitur apa yang harus dimasukkan, seberapa rapi sebelum disetujui, atau gaya penulisan kode apa yang dapat diterima, itu harus ditangani di luar konteks code review
• Namun dalam jenis situasi code review lain, tidak demikian. Terutama jika orang luar proyek mengirim patch yang tidak diminta, situasinya sangat berbeda
  • Skenario seperti ini umumnya terjadi di free software
  • Karena siapa pun di seluruh dunia bisa memodifikasi source code dan sebagian dari mereka mungkin mencoba mengirimkan kembali perubahan tersebut
• Tetapi ini juga bisa terjadi dalam situasi lain
  • Di dalam perusahaan yang mengembangkan kode proprietary, developer dari satu tim bisa mengirim patch ke proyek tim lain sambil berharap beruntung
• Dalam situasi seperti ini, kemungkinan penerima patch memang sejak awal tidak menginginkan perubahan itu, atau sama sekali tidak setuju dengan cara pelaksanaannya, sehingga patch tersebut mungkin tidak akan diterima sama sekali
  • Dalam kasus ini, itu bukan penyalahgunaan wewenang sementara yang diberikan sebagai reviewer sejawat, melainkan pelaksanaan yang sah atas wewenang permanen sebagai penanggung jawab proyek
  • Saya yang menentukan arah proyek perangkat lunak saya
• Ketika reviewer kode menjalankan peran 'gatekeeping' seperti ini, tidak selalu salah jika mereka mengkritik patch dengan alasan patch tersebut melanggar prinsip desain umum atau persyaratan yang sudah ada
  • Mungkin saya memang tidak tahu cara menyelesaikan masalah itu dengan cara yang sesuai dengan persyaratan
  • Bahkan bisa jadi justru itulah bagian sulitnya, dan satu-satunya alasan saya sendiri belum membuat perubahan yang sama
• Namun bahkan dalam konteks gatekeeping, menerapkan 'The Guessing Game' tanpa penjelasan tetaplah tidak sopan
  • Saya biasanya berusaha menjelaskan bahwa developer telah melewatkan bagian yang sulit, dan jika saya sendiri tidak tahu jawabannya, saya akan mengatakan itu
• Tentu saja, tidak ada alasan pembenaran untuk penghambatan pasif-agresif seperti 'The Death of a Thousand Round Trips'
  • Jika Anda benar-benar tidak ingin patch itu masuk ke kode sama sekali dan Anda memang memegang peran gatekeeping dengan wewenang yang sah untuk membuat keputusan itu, Anda bisa menjelaskannya dengan kata-kata agar pengirim tidak membuang waktu lebih lanjut

Disclaimer

• Selama bertahun-tahun saya mengumpulkan catatan untuk artikel ini dari code review yang saya ikuti (di kedua sisi), code review yang saya amati antara orang lain, dan code review yang hanya saya dengar lewat percakapan
• Jadi ini bukan ditujukan kepada orang tertentu yang baru-baru ini mereview kode saya

[intajon]

Ternyata ini mungkin bukan berlebihan....

[magulhalmom]

Ini benar-benar pernah saya alami, dan saya hampir berhenti jadi developer. Benar-benar sulit untuk bangkit lagi.

[bungker]

Baca tulisan ini hampir bikin PTSD kambuh.

[kayws426]

Sepertinya Anda sudah mengumpulkan catatan untuk artikel ini dengan baik selama ini!!

[gongsil1212]

Bahkan hanya membacanya saja sudah terasa seperti pelecehan mental...

[wedding]

Jadi intinya ada di baris terakhir, ya? wkwkwk.....

[jypark]

Wah.. rasanya sampai mau kena kanker;;

[ahwjdekf]

Hal seperti ini juga cukup sering terlihat di Korea kalau pergi ke tempat yang melakukan si + sm. Biasanya ini disebut perundungan wilayah. Orang-orang jahat melakukan berbagai macam cara untuk menjaga mata pencaharian mereka.

[kenuheo]

Ternyata ada banyak cara yang licik.

[gooksangom]

Dalam jangka panjang, orang yang melakukan hal seperti itu tanpa alasan yang masuk akal biasanya akan 1) lebih cepat tersingkir dari jaringan pergaulan developer, atau 2) kalau mereka sangat kompeten sampai memegang porsi besar meski kepribadiannya buruk dan karena itu sulit disingkirkan, kondisi itu bertahan karena ada seseorang yang berperan sebagai adapter dan menjadi jalur komunikasi untuk menjaga koneksinya; jadi kalau orang tengah itu hilang karena alasan apa pun, tidak akan lama sebelum dia benar-benar disingkirkan. Bagaimanapun juga, pada akhirnya uang baru bisa berputar kalau orang-orang berkumpul dan mengerjakan sesuatu bersama, dan kalau uang berputar maka orang juga berpindah-pindah; karena itu orang yang tidak bisa menjaga hubungan baik dengan orang lain pada akhirnya akan tersisih dan tertinggal dari kelompok.

Biasanya, orang yang bertahan lama dalam kelompok meski karakternya buruk sering salah paham dan mengira dirinya bertahan karena dia semacam sosok luar biasa seperti Sherlock versi drama, semacam high-functioning sociopath, padahal kenyataannya cuma karena dia masih punya nilai guna sehingga orang-orang di sekitarnya menahan diri dan memanfaatkannya; begitu nilai gunanya hilang, hubungannya akan berubah jadi "senang pernah bekerja sama, dan jangan pernah bertemu lagi". Sherlock yang diperankan Cumberbatch pun bagi kita dari luar memang tampak seperti sociopath yang menarik, tetapi kalau tidak ada orang-orang di sekitarnya yang tidak menyerah padanya dan tetap peduli padanya, tidak akan ada cerita apa-apa.

[dokdok]

Orang yang tetap bertahan lama meski kepribadiannya buruk sering kali keliru mengira bahwa mereka bertahan karena dirinya sesuatu yang hebat, semacam sosiopat berfungsi tinggi ala Sherlock dalam drama, padahal sebenarnya orang-orang di sekitarnya hanya menahan diri dan memanfaatkannya karena masih ada nilainya. Begitu nilai guna itu hilang, hubungan itu berubah menjadi, "Senang rasanya pernah bersama dan semoga kita tak bertemu lagi." ==> Ini kalimat yang luar biasa. Harus saya ingat.

[savvykang]

Ini mengingatkan saya pada perundungan di tempat kerja atau power harassment.

[xguru]

Katanya humor, tapi pas baca tulisannya rasanya langsung naik darah..