Pedoman dan praktik yang baik untuk tinjauan kode wajib [ditutup]

Kami mencoba peninjauan kode wajib pada setiap komit - tidak ada yang masuk ke master yang belum divalidasi oleh setidaknya 1 orang bukan penulis - untuk beberapa sprint. Kami telah menerima dari pengembang dan manajemen (yang merupakan situasi yang luar biasa) dan kami ingin mendapatkan beberapa manfaat yang dikenal untuk:

• pengurangan bug yang jelas
• lebih banyak kesadaran akan perubahan yang terjadi di sekitar proyek
• "Aku tahu seseorang akan melihat ini jadi aku tidak akan malas" / efek anti-koboi
• peningkatan konsistensi dalam / lintas proyek

Tetapi kami memperkenalkan sesuatu yang dikenal untuk mengurangi kecepatan, dan jika dilakukan dengan salah dapat membuat langkah birokrasi bodoh dalam pipa komit yang tidak melakukan apa pun selain memakan waktu. Hal-hal yang saya khawatirkan:

• ulasan beralih hanya menjadi nit picking
• (Secara hiperbola) orang-orang membuka masalah arsitektur besar sebagai bagian dari review dua baris.
• Saya tidak ingin bias jawaban dengan hal lain.

Meskipun kita semua orang yang beralasan dan kita akan melakukan banyak analisis diri, kita pasti bisa menggunakan beberapa wawasan yang memenangkan pertempuran tentang hal-hal apa yang harus kita capai dalam sesi peninjauan untuk benar-benar membuat ulasan bekerja untuk kita . Apa saja pedoman dan kebijakan yang menurut Anda berhasil?

1. Buat ulasan singkat.

   Sulit untuk tetap terkonsentrasi, terutama selama tinjauan kode, untuk waktu yang lama. Selain itu, tinjauan kode panjang dapat mengindikasikan bahwa ada terlalu banyak hal untuk dikatakan pada kode (lihat dua poin berikutnya) atau bahwa tinjauan menjadi diskusi tentang masalah yang lebih besar, seperti arsitektur.

   Juga, ulasan harus tetap review, bukan diskusi. Itu tidak berarti bahwa pembuat kode tidak dapat membalas, tetapi seharusnya tidak berubah menjadi pertukaran pendapat yang panjang.

2. Hindari meninjau kode yang terlalu buruk.

   Meninjau kode berkualitas rendah menyedihkan bagi peninjau dan penulis kode. Jika sepotong kode mengerikan, tinjauan kode tidak berguna. Sebagai gantinya, penulis harus diminta untuk menulis ulang kode dengan benar.

3. Gunakan checker otomatis sebelum ulasan.

   Checker otomatis menghindari pemborosan waktu menunjuk kesalahan yang dapat ditemukan secara otomatis. Misalnya, untuk kode C #, menjalankan StyleCop, metrik Kode dan terutama analisis Kode adalah kesempatan yang baik untuk menemukan beberapa kesalahan sebelum ditinjau. Kemudian, review kode dapat dihabiskan untuk poin yang sangat sulit dilakukan untuk mesin.

4. Pilih dengan cermat orang-orang yang melakukan tinjauan.

   Dua orang yang tidak tahan satu sama lain tidak akan melakukan review yang baik terhadap kode yang lain. Masalah yang sama muncul ketika satu orang tidak menghormati yang lain (tidak peduli apakah itu resensi atau penulis, omong-omong).

   Selain itu, beberapa orang tidak dapat melihat kode mereka ditinjau, sehingga mereka memerlukan pelatihan dan persiapan khusus untuk memahami bahwa mereka tidak dikritik dan mereka seharusnya tidak melihatnya sebagai sesuatu yang negatif. Melakukan tinjauan, tidak siap, tidak akan membantu, karena mereka akan selalu bersikap defensif dan tidak akan mendengarkan kritik terhadap kode mereka (menganggap setiap saran sebagai kritik).

5. Lakukan tinjauan formal dan informal.

   Memiliki daftar periksa membantu dalam berkonsentrasi pada satu set kelemahan yang tepat, menghindari beralih ke pengambilan nit. Daftar periksa ini dapat berisi poin-poin seperti:

   • Injeksi SQL,
   • Asumsi yang salah tentang bahasa yang dapat menyebabkan kesalahan,
   • Situasi khusus yang dapat menyebabkan kesalahan, seperti prioritas operator. Misalnya, dalam C #, var a = b ?? 0 + c ?? 0;mungkin terlihat bagus untuk seseorang yang ingin menambahkan dua angka yang dapat dibatalkan dengan menyatu di nol, tetapi ternyata tidak.
   • Deallokasi memori,
   • Pemuatan malas (dengan dua risiko: memuat hal yang sama lebih dari satu kali, dan tidak memuatnya sama sekali),
   • Meluap,
   • Struktur data (dengan kesalahan seperti daftar sederhana alih-alih kumpulan hash, misalnya),
   • Input validasi dan pemrograman defensif secara umum,
   • Keamanan benang,
   • dll.

   Saya menghentikan daftar di sini, tetapi ada ratusan poin yang dapat muncul dalam daftar periksa, tergantung pada titik lemah dari penulis yang tepat.

6. Sesuaikan daftar periksa secara progresif.

   Agar tetap konstruktif dan berguna dari waktu ke waktu, daftar periksa yang digunakan dalam tinjauan formal harus disesuaikan dari waktu ke waktu tergantung pada kesalahan yang ditemukan. Sebagai contoh, tinjauan informal pertama dapat mengungkapkan sejumlah risiko SQL Injection. Pemeriksaan SQL Injection akan dimasukkan dalam daftar periksa. Ketika, beberapa bulan kemudian, tampak bahwa penulis sekarang sangat berhati-hati tentang permintaan dinamis vs parametrized, SQL Injection dapat dihapus dari daftar periksa.

Kami hampir menyukai daftar periksa:

• Tunjukkan padaku deskripsi tugas.
• Arahkan saya pada hasilnya, dan tunjukkan itu berhasil. Jalankan skenario yang berbeda (input tidak valid, dll).
• Tunjukkan padaku tes kelulusan. Seperti apa cakupan tes?
• Tunjukkan kodenya - di situlah kami mencari inefisiensi yang jelas.

Bekerja dengan cukup baik.

Saya pikir seseorang yang memiliki kekuasaan atas yang lain akan cukup, administrator atau moderator untuk memotong komentar yang tidak relevan, mempercepat meninjau hal-hal yang perlu ditinjau cepat. Pembuat keputusan tunggal.

Kekurangannya adalah orang ini harus melakukan itu sebagai tugas utama, sementara dia bisa melakukan sesuatu yang lain, dan mungkin Anda ingin memiliki orang yang paling berpengalaman dalam posisi ini.

Hal kedua adalah mengotomatisasi sebanyak yang Anda bisa!

• mengendalikan ruang putih
• perangkat lunak pengontrol gaya
• pembuatan otomatis sebelum peninjauan kode
• pengujian otomatis sebelum tinjauan kode

Hal-hal itu akan menghapus setidaknya beberapa hal yang mungkin dikomentari orang tanpa kebutuhan nyata. Jika tidak membangun atau memiliki spasi putih, itu tidak cukup baik untuk ditinjau, diperbaiki dan terapkan untuk ditinjau kembali. Jika tidak membangun atau beberapa tes gagal maka jelas bahwa itu tidak cukup baik.

Banyak hal tergantung pada teknologi Anda, tetapi temukan apa yang dapat Anda periksa secara otomatis semakin banyak semakin baik.

Kami belum memenangkan pertempuran ini, tapi itulah yang kami temukan berguna.