Secara tradisional kami melakukan peninjauan kode sebelum melakukan, saya bertengkar dengan rekan saya hari ini, yang lebih memilih peninjauan kode setelah melakukan.
Pertama, inilah latar belakangnya,
- Kami memiliki beberapa pengembang berpengalaman dan kami juga memiliki karyawan baru dengan pengalaman pemrograman hampir nol.
- Kami ingin melakukan iterasi cepat dan pendek untuk merilis produk kami.
- Semua anggota tim berada di situs yang sama.
Keuntungan dari tinjauan kode sebelum melakukan yang saya pelajari:
- Mentor karyawan baru
- Cobalah untuk mencegah kesalahan, kegagalan, desain yang buruk di awal siklus pengembangan
- Belajar dari orang lain
- Cadangan pengetahuan jika seseorang berhenti
Tapi saya juga punya beberapa pengalaman buruk:
- Efisiensi rendah, beberapa perubahan dapat ditinjau selama beberapa hari
- Sulit untuk menyeimbangkan kecepatan dan kualitas, terutama bagi pemula
- Salah satu anggota tim merasa tidak percaya
Mengenai tinjauan pasca-komitmen, saya tahu sedikit tentang ini, tetapi hal yang paling saya khawatirkan adalah risiko kehilangan kendali karena kurangnya ulasan. Ada pendapat?
MEMPERBARUI:
- Kami menggunakan Perforce untuk VCS
- Kami memberi kode dan melakukan di cabang yang sama (cabang trunk atau bug fixing)
- Untuk meningkatkan efisiensi, kami telah mencoba memecah kode menjadi perubahan kecil. Kami juga telah mencoba beberapa tinjauan dialog langsung, tetapi tidak semua orang mengikuti aturan. Ini adalah masalah lain.
code-reviews
kelima
sumber
sumber
Jawaban:
Seperti yang dikatakan Simon Whitehead dalam komentarnya , itu tergantung pada strategi percabangan Anda.
Jika pengembang memiliki cabang pribadi mereka sendiri untuk pengembangan (yang saya sarankan dalam sebagian besar situasi), saya akan melakukan tinjauan kode sebelum bergabung dengan trunk atau repositori utama. Ini akan memungkinkan pengembang memiliki kebebasan untuk check-in sesering yang mereka inginkan selama siklus pengembangan / pengujian, tetapi kode waktu apa pun yang masuk ke cabang yang berisi kode yang dikirimkan, akan ditinjau.
Secara umum, pengalaman buruk Anda dengan ulasan kode terdengar lebih seperti masalah dengan proses peninjauan yang memiliki solusi. Dengan meninjau kode dalam potongan yang lebih kecil dan terpisah, Anda dapat memastikan tidak terlalu lama. Angka yang baik adalah bahwa 150 baris kode dapat ditinjau dalam satu jam, tetapi nilainya akan lebih lambat untuk orang yang tidak terbiasa dengan bahasa pemrograman, sistem yang sedang dikembangkan, atau kritikalitas sistem (kritikan keselamatan membutuhkan lebih banyak waktu) - informasi ini mungkin berguna untuk meningkatkan efisiensi dan memutuskan siapa yang berpartisipasi dalam tinjauan kode.
sumber
Ada mantra yang sepertinya belum ada yang dikutip: Periksa awal dan sering :
Saya telah bekerja untuk beberapa perusahaan yang memiliki pendekatan berbeda terhadap ini. Satu mengizinkannya, selama itu tidak mencegah kompilasi. Yang lain akan panik jika Anda memeriksa bug sama sekali. Yang pertama lebih disukai. Anda harus berkembang dalam semacam lingkungan yang memungkinkan Anda untuk berkolaborasi dengan orang lain dalam hal-hal yang masih dalam proses, dengan pemahaman bahwa semuanya akan diuji nanti.
Ada juga pernyataan Jeff Atwood: Jangan takut untuk merusak barang-barang :
Saya juga akan menambahkan itu untuk ulasan rekan, jika seseorang ingin Anda mengubah sesuatu, memiliki riwayat versi asli Anda dalam sumber adalah alat pembelajaran yang sangat berharga.
sumber
Saya baru-baru ini mulai melakukan tinjauan pra-komitmen dalam proyek yang saya ikuti dan saya harus mengatakan bahwa saya sangat terkejut tentang betapa tidak masalahnya itu.
Kelemahan terbesar dari ulasan pasca-komit yang saya lihat adalah bahwa seringkali hanya urusan satu orang: Seseorang meninjau kode untuk kebenaran, tetapi penulis biasanya tidak terlibat kecuali ada kesalahan serius. Masalah kecil, saran atau petunjuk biasanya tidak sampai ke penulis sama sekali.
Ini juga mengubah hasil yang dirasakan dari ulasan kode: itu dilihat sebagai sesuatu yang hanya menghasilkan pekerjaan tambahan, sebagai lawan dari sesuatu di mana setiap orang (peninjau dan penulis kode) dapat mempelajari hal-hal baru setiap saat.
sumber
Ulasan kode pra-komit tampak begitu alami, tidak pernah terpikir oleh saya bahwa ulasan dapat dengan sengaja dilakukan setelah melakukan. Dari perspektif integrasi berkelanjutan, Anda ingin mengkomit kode Anda setelah selesai, bukan dalam kondisi kerja-tapi-mungkin-buruk-ditulis, kan?
Mungkin itu karena cara kami selalu melakukannya di tim saya adalah dialog langsung yang diprakarsai oleh pengembang asli, bukan ulasan yang tidak sinkron, berdasarkan kontrol, berbasis dokumen.
sumber
Sebagian besar repositori saat ini mendukung komit dua fase atau rak (cabang pribadi, permintaan tarik, pengajuan tambalan atau apa pun yang Anda ingin menyebutnya), yang akan memungkinkan Anda untuk memeriksa / meninjau pekerjaan sebelum menariknya ke jalur utama. Saya akan mengatakan bahwa meningkatkan alat-alat ini akan memungkinkan Anda untuk selalu melakukan tinjauan pra-komit.
Selain itu, Anda dapat mempertimbangkan pengkodean pasangan (pasangan senior dengan junior) sebagai cara lain untuk memberikan tinjauan kode bawaan. Anggap saja sebagai pemeriksaan kualitas pada jalur perakitan, bukan setelah mobil meluncur.
sumber
Lakukan keduanya :
sumber
Setiap tinjauan formal harus dilakukan pada file sumber yang berada di bawah kendali konfigurasi, dan catatan review dengan jelas menyatakan revisi file.
Ini menghindari argumen tipe "Anda belum mendapatkan file terbaru", dan memastikan semua orang meninjau salinan kode-sumber yang sama.
Ini juga berarti bahwa, jika diperlukan koreksi pasca-peninjauan, sejarah dapat dijelaskan dengan fakta itu.
sumber
Untuk ulasan kode itu sendiri, suara saya untuk 'selama' komit.
Sebuah sistem seperti gerrit atau semanggi (saya pikir) dapat melakukan perubahan dan kemudian meminta reviewer mengkomitnya ke kontrol sumber (push in git) jika itu baik. Itu yang terbaik dari kedua dunia.
Jika itu tidak praktis, saya pikir setelah berkomitmen adalah kompromi terbaik. Jika desainnya bagus maka hanya pengembang paling junior yang harus memiliki hal-hal yang cukup buruk Anda tidak ingin mereka pernah berkomitmen. (Buat ulasan pra-komitmen untuk mereka).
Yang mengarah pada tinjauan desain - selagi Anda bisa melakukannya pada waktu tinjauan kode (atau dalam hal ini pada waktu penyebaran pelanggan), menemukan masalah desain harus dilakukan lebih awal dari itu - sebelum kode tersebut ditulis.
sumber
Dengan peer review ada risiko minimal kehilangan kendali. Setiap saat dua orang memiliki pengetahuan tentang kode yang sama. Mereka harus beralih sesekali, jadi mereka harus penuh perhatian setiap saat untuk melacak kode.
Masuk akal untuk memiliki pengembang yang terampil dan seorang pemula yang bekerja bersama. Sepintas ini tampaknya tidak efisien, tetapi ternyata tidak. Bahkan, ada lebih sedikit bug, dan butuh waktu lebih sedikit untuk memperbaikinya. Selain itu, para pemula akan belajar lebih cepat.
Apa yang mencegah desain buruk, ini harus dilakukan sebelum pengkodean. Jika ada perubahan besar / peningkatan / desain baru, itu harus ditinjau sebelum pengkodean dimulai. Ketika desain benar-benar dikembangkan, tidak banyak yang bisa dilakukan. Meninjau kode akan lebih mudah dan membutuhkan waktu lebih sedikit.
Saya setuju bahwa tidak penting untuk meninjau kode sebelum melakukan hanya jika kode tersebut diproduksi oleh pengembang yang berpengalaman, yang telah membuktikan keahlian mereka. Tetapi jika ada seorang pemula, kode harus ditinjau sebelum melakukan: peninjau harus duduk di sebelah pengembang dan menjelaskan setiap perubahan atau peningkatan yang dilakukan oleh mereka.
sumber
Ulasan mendapat manfaat dari komitmen sebelum dan sesudah.
Komitmen pra-ulasan
Tidak Menjalankan Komitmen Selama Review
Saya telah menggunakan alat Atlassian dan telah melihat menjalankan komit terjadi selama peninjauan. Ini membingungkan bagi pengulas, jadi saya sarankan untuk tidak melakukannya.
Posting Revisi Ulasan
Setelah pengulas menyelesaikan umpan balik mereka, baik secara lisan maupun tertulis, moderator harus memastikan penulis membuat perubahan yang diminta. Terkadang pengulas atau penulis mungkin tidak setuju apakah akan menunjuk item ulasan sebagai kesalahan, saran, atau investigasi. Untuk menyelesaikan ketidaksepakatan dan memastikan item investigasi dibersihkan dengan benar, tim peninjau bergantung pada penilaian moderator.
Pengalaman saya dengan sekitar 100 inspeksi kode adalah bahwa ketika pengulas dapat mereferensikan standar pengkodean yang tidak ambigu, dan untuk sebagian besar jenis logika dan kesalahan pemrograman lainnya, hasil tinjauan umumnya dipotong dengan jelas. Kadang-kadang ada perdebatan tentang nit-picking atau titik gaya dapat berubah menjadi argumen. Namun, memberikan kekuatan keputusan kepada moderator menghindari kebuntuan.
Komitmen Pasca-Tinjauan
sumber
Itu tergantung pada tim Anda. Untuk tim yang relatif berpengalaman yang bagus tentang hal kecil, sering melakukan kemudian review pasca-komitmen hanya untuk mendapatkan sepasang mata kedua pada kode baik-baik saja. Untuk komit yang lebih besar, lebih kompleks, dan / atau untuk pengembang yang kurang berpengalaman, maka pra-komit ulasan untuk memperbaiki masalah sebelum mereka masuk tampaknya lebih bijaksana.
Sejalan dengan itu, memiliki proses CI yang baik dan / atau check-in yang terjaga keamanannya mengurangi kebutuhan untuk ulasan sebelum melakukan (dan bisa dibilang posting komit untuk banyak dari mereka).
sumber
Saya dan kolega saya melakukan riset ilmiah tentang topik ini baru-baru ini, jadi saya ingin menambahkan beberapa wawasan kami meskipun ini pertanyaan yang agak lama. Kami membangun model simulasi proses / tim pengembangan Kanban yang gesit dan membandingkan tinjauan pra-komit dan pasca-komit untuk sejumlah besar situasi yang berbeda (jumlah anggota tim yang berbeda, tingkat keterampilan yang berbeda, ...). Kami melihat hasil setelah 3 tahun waktu pengembangan (disimulasikan), dan mencari perbedaan sehubungan dengan efisiensi (poin cerita jadi), kualitas (bug ditemukan oleh pelanggan) dan waktu siklus (waktu dari awal hingga pengiriman cerita pengguna) . Temuan kami adalah sebagai berikut:
Dari ini, kami memperoleh aturan heuristik berikut:
Makalah penelitian lengkap tersedia di sini: http://dx.doi.org/10.1145/2904354.2904362 atau di situs web saya: http://tobias-baum.de
sumber
Menurut pendapat saya, review kode rekan bekerja paling baik jika dilakukan pasca-komit.
Saya akan merekomendasikan menyesuaikan strategi percabangan Anda. Menggunakan cabang pengembang atau cabang fitur memiliki sejumlah manfaat ... yang paling tidak memfasilitasi tinjauan kode pasca-komitmen.
Alat seperti Crucible akan memperlancar dan mengotomatiskan proses peninjauan. Anda dapat memilih satu atau beberapa perubahan yang berkomitmen untuk dimasukkan dalam ulasan. Crucible itu akan menampilkan file mana yang disentuh dalam perubahan yang dipilih, melacak file mana yang sudah dibaca masing-masing reviewer (menunjukkan% selesai secara keseluruhan) dan membiarkan pengulas dengan mudah membuat komentar.
http://www.atlassian.com/software/crucible/overview
Beberapa manfaat lain dari cabang pengguna / fitur:
Untuk pengembang yang tidak berpengalaman, konsultasi rutin dengan mentor dan / atau pemrograman pasangan adalah ide yang bagus, tapi saya tidak akan menganggap ini sebagai "tinjauan kode".
sumber
Kedua. (Agak.)
Anda harus meninjau kode Anda sendiri sebelum menyimpulkannya. Di Git, saya pikir area pementasannya bagus. Setelah saya melakukan perubahan, saya berlari
git diff --cached
untuk melihat semua yang dipentaskan. Saya menggunakan ini sebagai kesempatan untuk memastikan saya tidak memeriksa semua file yang bukan miliknya (membangun artefak, log, dll.) Dan memastikan bahwa saya tidak memiliki kode debug di sana atau kode penting apa pun yang dikomentari di luar. (Jika saya melakukan sesuatu yang saya tahu saya tidak ingin check-in, saya biasanya meninggalkan komentar dalam semua batas sehingga saya akan mengenalinya selama pementasan.)Karena itu, tinjauan kode sejawat Anda harus dilakukan secara umum setelah Anda berkomitmen, dengan asumsi bahwa Anda sedang mengerjakan cabang topik. Ini adalah cara termudah untuk memastikan bahwa semua orang meninjau hal yang benar, dan jika ada masalah besar, maka itu bukan masalah besar untuk memperbaikinya di cabang Anda atau menghapusnya dan memulai lagi. Jika Anda melakukan tinjauan kode secara tidak sinkron (yaitu menggunakan Google Code atau Atlassian Crucible), maka Anda dapat dengan mudah beralih cabang dan mengerjakan sesuatu yang lain tanpa harus secara terpisah melacak semua patch / diff yang berbeda yang saat ini sedang ditinjau selama beberapa hari.
Jika Anda tidak mengerjakan cabang topik, Anda harus melakukannya . Ini mengurangi stres dan kerumitan dan membuat perencanaan rilis jauh lebih sedikit stres dan rumit.
Sunting: Saya juga harus menambahkan bahwa Anda harus melakukan peninjauan kode setelah pengujian, yang merupakan argumen lain yang mendukung melakukan kode terlebih dahulu. Anda tidak ingin kelompok uji Anda bermain-main dengan puluhan tambalan / perbedaan dari semua programmer, lalu mengajukan bug hanya karena mereka menerapkan tambalan yang salah di tempat yang salah.
sumber
Pemrograman berpasangan 100% (tidak peduli seberapa senior Anda berpikir Anda) dengan banyak komit kecil dan sistem CI yang dibangun di atas SETIAP komit (dengan pengujian otomatis termasuk unit, integrasi dan fungsional sedapat mungkin). Ulasan pasca-komitmen untuk perubahan besar atau berisiko. Jika Anda harus memiliki semacam peninjauan gated / pra-komitmen, Gerrit berfungsi.
sumber
Keuntungan dari tinjauan kode pada saat check in (buddy check) adalah umpan balik segera, sebelum potongan kode besar telah selesai.
Kerugian dari tinjauan kode pada saat check-in adalah bahwa hal itu dapat mencegah orang dari check-in sampai panjangnya kode telah selesai. Jika ini terjadi, itu sepenuhnya meniadakan keuntungan.
Apa yang membuat ini lebih sulit adalah tidak semua pengembang sama. Solusi sederhana tidak berfungsi untuk semua programmer . Solusi sederhana adalah:
Pemrograman pasangan yang diamanatkan, yang memungkinkan untuk melakukan check-in yang sering karena buddy ada di sebelah Anda. Ini mengabaikan bahwa pemrograman pasangan tidak selalu bekerja untuk semua orang. Dilakukan dengan benar, pemrograman pasangan juga bisa sangat melelahkan sehingga tidak harus dilakukan sepanjang hari.
Cabang pengembang, kode hanya ditinjau dan diperiksa di cabang utama ketika selesai. Beberapa pengembang cenderung untuk bekerja dalam kerahasiaan, dan setelah seminggu mereka menemukan beberapa kode yang mungkin atau mungkin tidak lolos peninjauan karena masalah mendasar yang mungkin terlihat sebelumnya.
Tinjau setiap check-in, yang menjamin sering ditinjau. Beberapa pengembang pelupa dan mengandalkan sangat sering check-in, yang berarti orang lain harus melakukan tinjauan kode setiap 15 menit.
Tinjau pada waktu yang tidak ditentukan setelah check-in. Ulasan akan didorong lebih jauh ketika ada tenggat waktu. Kode yang tergantung pada sudah berkomitmen tetapi belum ditinjau kode akan dilakukan. Ulasan akan menandai masalah dan masalah akan diletakkan di backlog untuk diperbaiki "nanti". Ok, saya berbohong: Ini bukan solusi yang sederhana, itu bukan solusi sama sekali. Tinjau pada waktu tertentu setelah karya check-in, tetapi kurang sederhana karena Anda harus memutuskan apa waktu yang ditentukan
Dalam praktiknya, Anda membuat karya ini dengan membuatnya lebih sederhana, dan lebih kompleks pada saat bersamaan. Anda menetapkan pedoman sederhana, dan membiarkan setiap tim pengembangan menentukan sebagai tim apa yang harus mereka lakukan untuk mengikuti panduan ini. Contoh pedoman tersebut adalah:
Banyak bentuk alternatif dari pedoman tersebut dimungkinkan. Fokus pada apa yang sebenarnya Anda inginkan (kode yang diulas bersama, kemajuan pekerjaan yang dapat diamati, pertanggungjawaban) dan biarkan tim mencari tahu bagaimana mereka dapat memberikan apa yang mereka inginkan.
sumber
kami benar-benar melakukan hybrid di LedgerSMB. Committer melakukan perubahan yang ditinjau setelahnya. Non-committer mengirimkan perubahan ke committer untuk ditinjau sebelumnya. Ini cenderung berarti dua tingkatan ulasan. Pertama, Anda mendapatkan mentor untuk meninjau dan membimbing Anda. Kemudian mentor itu membuat kode ditinjau untuk kedua kalinya setelah dia menandatangani dan umpan balik bersirkulasi. Pengendara baru biasanya menghabiskan banyak waktu untuk meninjau komitmen orang lain pada awalnya.
Ini bekerja dengan cukup baik. Masalahnya adalah review setelah biasanya lebih sepintas daripada review sebelumnya, jadi Anda ingin memastikan bahwa ulasan setelah disediakan untuk mereka yang telah membuktikan diri. Tetapi jika Anda memiliki review dua tingkat untuk orang-orang baru itu berarti bahwa masalah lebih mungkin untuk ditangkap dan diskusi.
sumber
Komit dimana? Ada cabang yang saya buat untuk melakukan beberapa pekerjaan. Saya berkomitmen pada cabang itu kapan pun saya mau. Itu bukan urusan siapa-siapa. Kemudian di beberapa titik cabang itu diintegrasikan ke dalam cabang pengembangan. Dan di suatu tempat di antaranya adalah ulasan kode.
Ulasan resensi jelas setelah saya berkomitmen untuk cabang saya. Dia tidak duduk di meja saya, jadi dia tidak bisa memeriksanya sebelum saya berkomitmen ke cabang saya . Dan dia meninjau sebelum penggabungan dan berkomitmen untuk cabang pengembangan.
sumber
Hanya saja, jangan lakukan review kode sama sekali. Entah Anda percaya pengembang Anda mampu menulis kode yang baik, atau Anda harus menyingkirkannya. Kesalahan dalam logika harus ditangkap oleh tes otomatis Anda. Kesalahan adalah gaya harus ditangkap oleh alat analisis serat dan statis.
Mempunyai manusia yang terlibat dalam apa yang seharusnya menjadi proses otomatis adalah pemborosan.
sumber