Tinjau sebelum atau setelah kode melakukan, mana yang lebih baik?

71

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,

  1. Kami memiliki beberapa pengembang berpengalaman dan kami juga memiliki karyawan baru dengan pengalaman pemrograman hampir nol.
  2. Kami ingin melakukan iterasi cepat dan pendek untuk merilis produk kami.
  3. Semua anggota tim berada di situs yang sama.

Keuntungan dari tinjauan kode sebelum melakukan yang saya pelajari:

  1. Mentor karyawan baru
  2. Cobalah untuk mencegah kesalahan, kegagalan, desain yang buruk di awal siklus pengembangan
  3. Belajar dari orang lain
  4. Cadangan pengetahuan jika seseorang berhenti

Tapi saya juga punya beberapa pengalaman buruk:

  1. Efisiensi rendah, beberapa perubahan dapat ditinjau selama beberapa hari
  2. Sulit untuk menyeimbangkan kecepatan dan kualitas, terutama bagi pemula
  3. 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:

  1. Kami menggunakan Perforce untuk VCS
  2. Kami memberi kode dan melakukan di cabang yang sama (cabang trunk atau bug fixing)
  3. 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.
kelima
sumber
13
Apakah mereka berkomitmen ke cabang mereka sendiri? Itu mungkin argumen kolega Anda untuk ulasan pasca-komitmen. Secara pribadi saya akan mengatakan pra-komit untuk pengembang yang sangat tidak berpengalaman.
Simon Whitehead
Ulasan sebaliknya itu pilihan terbaik
shabunc
1
Bagaimana dengan keduanya? Selama mereka diidentifikasi dengan jelas itu seharusnya tidak menjadi masalah, misalnya cabang sebelum ditinjau, bergabung setelahnya. Ini memberikan akses langsung ke pengembang lain yang mungkin perlu melihat apa yang terjadi. Itu membuat perubahan terus-menerus yang dihasilkan dari ulasan, bantuan yang mudah bagi mereka yang dibimbing. Berbagai ulasan dapat diambil secara terpisah, misalnya, fungsional, keamanan, dan legal.
HABO

Jawaban:

62

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.

Thomas Owens
sumber
2
Jika pengembang memiliki cabang sendiri dan Anda memiliki alat peninjauan kode yang tepat, Anda dapat mempertahankan kontrol. Reviewer harus mencatat dalam alat apakah mereka sudah melakukan review atau tidak.
MarkJ
1
Harus ditambahkan bahwa memiliki komit yang dapat ditinjau menyiratkan bahwa pembuat kode sendiri memiliki pikiran yang lebih jernih, menegakkan fakta bahwa setiap masalah harus ditangani secara terpisah dalam langkah-langkah kecil yang berhasil. Ini memperketat loop umpan balik dan tampaknya suatu keharusan bagi tim tangkas mana pun.
vaab
@ Thomas, Perforce adalah alat VCS kami saat ini, kami semua kode dan komit di cabang yang sama, misalnya, semua di trunk atau rilis cabang. Saya mengerti apa yang Anda katakan, jika kita menjalankan Git, saya akan setuju ide Anda bahwa kebijakan peninjauan tergantung pada strategi percabangan.
Kelima
4
+1, ini bekerja lebih baik ketika masing-masing pengembang tidak memiliki cabang sendiri, tetapi ketika Anda menggunakan cabang fitur sebagai gantinya. Kami melakukan perbaikan bug langsung ke trunk, karena biasanya kecil, tetapi fitur masuk ke cabang mereka sendiri, mendapatkan banyak komitmen, lalu dapat ditinjau sebelum digabungkan ke trunk.
Izkata
1
@ThomasOwens: Perforce mendukung percabangan, tetapi tidak dengan kemudahan SVN, GIT, atau Mercurial.
kevin cline
35

Ada mantra yang sepertinya belum ada yang dikutip: Periksa awal dan sering :

Pengembang yang bekerja untuk waktu yang lama - dan lebih lama maksud saya lebih dari satu hari - tanpa memeriksa apa pun ke dalam kontrol sumber sedang menyiapkan diri mereka untuk beberapa integrasi serius yang sulit di masa depan. Damon Poole setuju :

Pengembang sering menunda check-in. Mereka menunda karena mereka tidak ingin mempengaruhi orang lain terlalu dini dan mereka tidak ingin disalahkan karena melanggar pembangunan. Tapi ini mengarah ke masalah lain seperti kehilangan pekerjaan atau tidak bisa kembali ke versi sebelumnya.

Aturan praktis saya adalah "check-in lebih awal dan sering", tetapi dengan peringatan bahwa Anda memiliki akses ke versi pribadi. Jika check-in segera terlihat oleh pengguna lain, maka Anda berisiko memperkenalkan perubahan yang tidak matang dan / atau merusak build.

Saya lebih suka memeriksa bagian-bagian kecil secara berkala daripada pergi dalam waktu lama tanpa tahu apa pun yang ditulis rekan kerja saya. Sejauh yang saya ketahui, jika kode tidak diperiksa ke dalam kontrol sumber, itu tidak ada . Saya kira ini adalah bentuk lain dari Don't Go Dark ; kode tidak terlihat sampai ada di repositori dalam beberapa bentuk.

... Jika Anda belajar untuk check-in lebih awal dan sering check-in, Anda akan punya cukup waktu untuk umpan balik, integrasi, dan ulasan sepanjang jalan ...

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 :

Cara paling langsung untuk meningkatkan sebagai pengembang perangkat lunak adalah benar-benar tidak takut ketika harus mengubah kode Anda. Pengembang yang takut kode rusak adalah pengembang yang tidak akan pernah matang menjadi profesional.

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.

lorddev
sumber
1
Saya suka jawaban ini - saya pikir ini mengisi topik yang tersisa yang disebutkan dalam karunia dengan cukup baik.
Joris Timmermans
penjelasan yang cukup menarik tentang mengapa penting untuk menghindari komitmen VCS yang diblokir oleh ulasan
nyamuk
1
Ini jauh lebih baik. Itu mulai membuat pengembangan terdengar seperti perusahaan yang menghargai komunikasi dalam tim sebagai lawan dari sistem penghindaran menyalahkan mekanistik.
Ian
19

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.

Joachim Sauer
sumber
5
Ini menunjukkan bahwa masalah kecil atau saran dapat "diperbaiki" oleh pengulas, atau tidak sama sekali? Saya berharap bahwa komentar ulasan APAPUN diumpankan kembali ke penulis untuk mengatasi (atau menolak)
Andrew
8

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.

guillaume31
sumber
apakah sudah waktunya digunakan untuk ulasan dialog langsung? Apakah tim Anda meninjau semua kode?
Kelima
Kami tidak meninjau semua kode, tetapi cukup banyak semua yang setidaknya cukup kompleks.
guillaume31
3
Ini sepenuhnya tergantung pada apa yang Anda gunakan untuk SCM. Dengan git, membuat cabang baru, berkomitmen untuk itu dan mendorong perubahan itu adalah cara yang sangat alami untuk melakukan tinjauan kode.
kubi
8

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.

Michael Brown
sumber
3
Saya suka pair coding, tapi Mike, senior dan junior bukan pair coding, itu mentoring. Saya sangat menyarankan pendampingan, tetapi dua hal ini harus dibedakan karena alasan untuk / menentang, dan hasilnya, benar-benar berbeda antara pendampingan dan pemrograman pasangan. Lihat posting ke-4 di: c2.com/cgi/wiki?PairProgrammingDoubts juga c2.com/cgi/wiki?PairProgrammingIsDoneByPeers
Jimmy Hoffa
Tidak selalu. Orang junior dapat memiliki input. Atau perhatikan "kesalahan bodoh".
Jeanne Boyarsky
@ JeanneBoyarsky Saya tidak mengatakan tidak melakukannya, hanya saja dinamikanya berbeda dan hasilnya berbeda (bukan kode, maksud saya manfaat yang dihasilkan untuk keseluruhan proses). Juga jika orang "junior" memiliki jumlah masukan desain bermanfaat yang sama atau lebih besar secara tidak proporsional ketika dipasangkan dengan seseorang yang senior, saya akan berpendapat bahwa "junior" tidak begitu junior atau "senior" tidak begitu senior.
Jimmy Hoffa
Anda benar ... tapi saya pikir itu cara paling efektif untuk berbagi pengetahuan.
Michael Brown
@ MikeBrown - sementara saya setuju dengan argumen Anda di sini, "wiki" yang tertaut adalah salah satu hal terburuk yang pernah saya baca tentang pemrograman pasangan. Semua keberatan dan kekhawatiran dilenyapkan dengan tangan, mereka yang meragukannya pada dasarnya disebut sebagai retard sosial, dan manajemen menghina karena tidak ingin menerapkan metodologi baru yang radikal dalam proses mereka tanpa bukti empiris bahwa itu sebenarnya memberikan keuntungan bisnis. Ini setara dengan komentar YouTube untuk toksisitasnya. Saya tidak tahu bagaimana orang berpikir ini adalah hal yang baik untuk pemrograman pasangan, dan saya katakan itu sebagai seseorang yang menyukainya.
Davor Ždralo
7

Lakukan keduanya :

  • pre commit - lakukan tinjauan semacam ini ketika itu adalah sesuatu yang sangat penting, seperti potongan kode yang sangat dapat digunakan kembali, atau keputusan desain utama
  • komit posting - lakukan semacam ulasan ketika Anda ingin mendapatkan pendapat tentang sepotong kode yang mungkin ditingkatkan
BЈовић
sumber
5

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.

Andrew
sumber
3

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.

ptyx
sumber
2

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.

superM
sumber
2

Ulasan mendapat manfaat dari komitmen sebelum dan sesudah.

Komitmen pra-ulasan

  • Memberi kepercayaan pengulas bahwa mereka sedang meninjau revisi terbaru penulis.
  • Membantu memastikan semua orang meninjau kode yang sama.
  • Memberikan referensi untuk perbandingan setelah revisi yang dibuat dari item ulasan selesai.

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

  • Memberi moderator dan pengulas lainnya titik data untuk dibandingkan dengan komitmen pra-ulasan.
  • Memberikan metrik untuk menilai nilai dan keberhasilan peninjauan pada penghapusan cacat dan peningkatan kode.
PengembangDon
sumber
1

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).

Telastyn
sumber
1

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:

  • Perbedaan dalam hal efisiensi dan kualitas dapat diabaikan dalam banyak kasus. Ketika tidak, review pasca komit memiliki beberapa manfaat sehubungan dengan kualitas (pengembang lain bertindak sebagai "penguji beta"). Untuk efisiensi, pasca komit memiliki beberapa manfaat dalam tim kecil dan pra komit memiliki beberapa manfaat dalam tim besar atau tidak terampil.
  • Tinjauan pra-komitmen dapat menyebabkan waktu siklus yang lebih lama, ketika Anda memiliki situasi di mana dimulainya tugas-tugas dependen tertunda oleh ulasan.

Dari ini, kami memperoleh aturan heuristik berikut:

  • Ketika Anda memiliki proses peninjauan kode yang sudah mapan, jangan repot-repot mengubah, itu mungkin tidak sepadan dengan usaha
    • kecuali Anda memiliki masalah dengan waktu siklus => Alihkan ke Posting
    • Atau masalah yang tergelincir mengganggu pengembang Anda terlalu sering => Alihkan ke Pre
  • Jika Anda belum melakukan review
    • Gunakan pra-komitmen jika salah satu dari manfaat ini berlaku untuk Anda
      • Tinjauan pra-komit memungkinkan orang luar tanpa hak berkomitmen untuk berkontribusi pada proyek sumber terbuka
      • Jika berbasis alat, tinjauan pra-komitmen menegakkan beberapa disiplin ulasan dalam tim dengan kepatuhan proses yang lemah
      • Tinjauan pra-komit dengan mudah mencegah perubahan yang tidak ditinjau dari pengiriman, yang rapi untuk penerapan berkelanjutan / siklus rilis yang sangat singkat
    • Gunakan pra-komitmen jika tim Anda besar dan Anda dapat hidup dengan atau menghindari masalah dalam waktu siklus
    • Kalau tidak (mis. Tim industri kecil, terampil) menggunakan komit pos
  • Cari kombinasi yang memberi Anda manfaat dari kedua dunia (kami belum mempelajari ini secara formal)

Makalah penelitian lengkap tersedia di sini: http://dx.doi.org/10.1145/2904354.2904362 atau di situs web saya: http://tobias-baum.de

Tobias B.
sumber
Apakah model ini telah diverifikasi dengan data dunia nyata?
Peter
1
Model ini telah diverifikasi dengan data dunia nyata sampai tingkat tertentu (sangat terbatas). Masalah utama adalah bahwa untuk sebagian besar faktor input, kami tidak memiliki nilai yang diukur dalam proyek dunia nyata. Verifikasi utama dilakukan dengan menghadirkan model kepada beberapa praktisi. Dua dari mereka (satu dengan latar belakang sebelum dan satu untuk posting) memeriksanya lebih detail. Jika kami memiliki data kuantitatif yang lebih baik, kami mungkin tidak akan membangun model sama sekali, tetapi hanya menganalisis data.
Tobias B.
Terima kasih, itu menempatkan jawaban dalam perspektif dan karenanya membuatnya lebih berharga. +1
Peter
0

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:

  • Pengembang mendapatkan manfaat dari kontrol versi (membuat cadangan perubahan, memulihkan dari riwayat, perubahan berbeda) dengan lebih sedikit khawatir tentang kerusakan sistem untuk orang lain.
  • Perubahan yang menyebabkan cacat atau tidak selesai tepat waktu dapat dicadangkan, diprioritaskan kembali, atau ditangguhkan jika perlu.

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".

RMorrisey
sumber
Crucible sangat luar biasa, dan biaya hanya $ 10 untuk memulai. (Meskipun versi $ 10 hanya akan mengelola 5 repositori, yang berarti Anda mungkin dapat melampaui dengan cepat, dan langkah selanjutnya dari sana jauh lebih mahal. Sesuatu seperti $ 1k IIRC.)
Mark E. Haase
0

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 --cacheduntuk 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.

Mark E. Haase
sumber
0

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.

Eric Smalling
sumber
0

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:

  • Pekerjaan dipecah dalam tugas-tugas yang seharusnya memakan waktu kurang dari sehari.
  • Tugas belum selesai jika kode (jika ada) belum dicek.
  • Tugas belum selesai jika kode (jika ada) belum ditinjau.

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.

Peter
sumber
-1

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.

Chris Travers
sumber
-1

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.

gnasher729
sumber
-3

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.

Mike Roberts
sumber
2
Sayangnya, pertanyaannya adalah apakah akan melakukan tinjauan sebelum atau sesudah - bukan apakah akan melakukannya atau tidak. Jika Anda memiliki pendapat tentang sebelum / sesudah, silakan tambahkan itu.
Marco