Apa cara terbaik untuk meninjau kode sebelum berkomitmen ke trunk? (SVN)

Apa cara terbaik untuk meninjau kode sebelum berkomitmen ke trunk SVN? Satu ide yang saya pikirkan adalah meminta pengembang untuk melakukan kode ke cabang dan kemudian meninjau kode sambil menggabungkan revisi cabang ke dalam bagasi. Apakah ini praktik yang baik? Jika tidak, apa lagi yang bisa dilakukan untuk meninjau kode sebelum berkomitmen ke trunk?

Ada dua sekolah meskipun - apa yang Anda usulkan atau "tinjau sebelum melakukan". Sebagian besar perbedaan dapat dilihat sebagai negatif dan / atau positif. - mis. Tidak ada pelacakan perubahan yang disebabkan oleh ulasan - apakah Anda ingin melihat ini sebagai komitmen yang terpisah, atau apakah Anda hanya tertarik pada pekerjaan akhir?

Tinjau sebelum melakukan - tidak diperlukan percabangan (meskipun dapat dilakukan jika diinginkan), harus memberikan akses pengulas ke folder yang berfungsi. Kode dapat diubah selama dan setelah peninjauan tanpa pelacakan. Koreksi yang disebabkan oleh ulasan tidak muncul di repositori.

Review After Commit (pada cabang) - Perlu memutar cabang untuk setiap review (meskipun ini mungkin sudah dalam alur kerja). Kode yang diajukan untuk ditinjau tidak dapat diubah tanpa melacak perubahannya. Seseorang harus menggabungkan cabang yang ditinjau, dan melacak apa yang telah ditinjau dan yang belum.

Ini sangat tergantung pada budaya dan pengalaman tim. Apa model yang Anda percayai, apa tujuan utama ulasan? Saya pribadi lebih suka ulasan setelah komit, karena memungkinkan perubahan sebagai hasil ulasan dilacak. Kami sekarang menggunakan Git dan Gerrit karena keduanya memberikan keseimbangan yang bagus antara dua opsi.

1. Sebelum melakukan jalankan 'svn diff' untuk menghasilkan file patch.
2. Kirim file tambalan ke peninjau yang menerapkannya pada salinan trunk yang bersih.
3. Reviewer mengulas perubahan menggunakan penampil perbedaan pilihan.
4. Reviewer melakukan build dan menjalankan tes.
5. Jika semuanya berjalan baik, beri tahu pengembang bahwa mereka dapat memeriksa perubahannya. Jika
   ada masalah, pengembang kembali ke langkah 1.

Ada dunia yang ideal, dan kemudian ada dunia nyata.

Di dunia ideal , semua kode Anda diuji, sehingga Anda dapat yakin bahwa apa pun yang diperiksa akan berfungsi atau Anda akan tahu kode itu rusak karena gagal satu atau lebih pengujian. Plus, siapa pun yang tidak begitu berpengalaman akan dipasangkan dengan seseorang yang berpengalaman, sehingga peninjauan kode dilakukan saat itu juga (Anda berkomitmen saat Anda pergi).

Di dunia nyata , segalanya berbeda. Bisnis ingin perubahan itu hidup sekarangdan akan memberi tahu Anda, dengan wajah yang benar-benar lurus, bahwa ya, Anda akan punya waktu untuk membersihkan kode dan menambahkan kotak uji nanti. Anda mungkin tidak akan punya waktu untuk meninjau semua kode dan persentase kode yang dicakup oleh pengujian terus menurun. Alasan utama untuk tinjauan kode adalah agar pengembang junior belajar dari pengembang senior (ketika ada waktu untuk itu) dengan meminta orang yang lebih berpengalaman memeriksa perubahan dan menyarankan "cara yang lebih baik dalam melakukan sesuatu (TM)". Anda akan memiliki pengembang senior yang hanya melakukan kode yang tidak ditinjau. Bercabang hanya untuk tinjauan kode dan kemudian menggabungkan adalah buang-buang waktu. Salah satu cara untuk mengatasi masalah ini adalah dengan mendeklarasikan pertemuan tim rutin 2 jam mingguan (atau lebih) di mana Anda memilih satu atau dua perubahan terbaru yang dilakukan orang dengan pemberitahuan singkat dan membuat penulis mereka "mempresentasikan" pendekatan mereka dengan melihat kode bersama-sama pada proyektor atau sesuatu. Hal ini dapat menyebabkan beberapa diskusi yang menarik (biasanya agak di luar topik) tetapi umumnya meningkatkan pemahaman semua orang tentang cara melakukannya dengan benar. Ditambah tekanan karena mungkin harus mempresentasikan kode Anda membuat beberapa orang melakukannya dengan lebih baik ;-)

Atau Anda mungkin beruntung dan mulai bekerja di lingkungan dunia nyata di mana tidak terlalu sibuk, programmer sebenarnya dihargai untuk apa yang mereka lakukan daripada disalahgunakan, dan ada waktu untuk melakukan semuanya dengan benar. Dalam hal ini jawaban saya adalah: cobalah beberapa metode berbeda yang disarankan dalam jawaban di sini dan lihat mana yang cocok dengan tim Anda dan cara Anda bekerja paling baik.

Cabang harus bekerja dengan baik, berdasarkan pengalaman saya menggunakannya dalam ulasan pra-komit di pekerjaan sebelumnya.

_{Perhatikan saat itu, kami menggunakan ulasan pra-komit hanya untuk tambalan penting untuk kode kandidat rilis produksi, jadi tidak ada banyak cabang (perubahan rutin dilewatkan melalui ulasan pasca-komit).}

Karena Anda tampaknya akan menggunakan ulasan pra-komit untuk semua perubahan, Anda mungkin perlu mengelola sejumlah besar cabang. Jika Anda mengharapkan pengembang membuat satu perubahan "dapat ditinjau" per minggu, Anda akan memiliki sekitar 50 cabang setiap tahun untuk setiap pengembang dalam tim. Jika Anda menggunakan potongan pekerjaan yang lebih kecil - seperti yang membutuhkan waktu 1, 2, 3 ... hari - kalikan 50 dengan 2, 3, 5 ....

Berikut adalah beberapa pertimbangan lain yang perlu dipertimbangkan jika Anda menginginkannya dengan cara terbaik .

1. menangani kasus ketika kode blokir tertunda diperlukan untuk anggota tim lain

Menetapkan, memantau, dan menyelesaikan konflik terkait tenggat waktu tinjauan kode. Per ingatan saya akan ulasan pra-komitmen terhadap perubahan rutin yang saya tangani di salah satu proyek sebelumnya, tenggat waktu yang wajar adalah sekitar 3 hari dan waktu untuk mulai mengkhawatirkan adalah ketika tinjauan tidak selesai lebih dari 1 hari setelah pengiriman.

_{Sebagai perbandingan, pada ulasan pasca-komitmen, persyaratan ini jauh lebih santai (saya menggunakan tenggat waktu 2 minggu dan mulai mengkhawatirkan setelah 1 minggu) - tetapi karena Anda menargetkan ulasan pra-komitmen, ini mungkin tidak menarik.}

2. menggabungkan konflik ketika mengirimkan kode yang ditinjau

Apa yang harus dilakukan jika komit untuk kode yang ditinjau diblokir oleh perubahan yang bertentangan yang dilakukan oleh orang lain saat kode menunggu tinjauan?

Beberapa opsi yang perlu dipertimbangkan adalah

• gulung balik ke awal dan mengharuskan pengembang menerapkan kembali dan meninjau kembali perubahan
  Untuk kasus itu Anda mungkin perlu mengatasi dampak negatif pada semangat tim yang mungkin (akan!) miliki.
• meneruskan tanggung jawab penggabungan kepada anggota tim lain ("master gabungan")
  Untuk kasus itu, Anda juga perlu memutuskan apakah penggabungan per se harus melalui peninjauan pra-komitmen atau tidak - dan jika ya, lalu apa yang harus dilakukan jika yang bergabung pada gilirannya bertemu dengan konflik lain.
• abaikan perubahan yang dilakukan terhadap kode yang ditinjau pada tahap penggabungan.
  Untuk itu Anda mungkin perlu mengatasi dampak negatif terhadap semangat tim terkait dengan fakta bahwa kode yang dilakukan berbeda dari yang telah ditinjau.
• menemukan cara untuk menghindari konflik.
  Pendekatan langsung adalah memungkinkan hanya satu pengembang pada saat yang sama untuk memodifikasi set file tertentu - meskipun ini tidak akan melindungi Anda dari jenis perubahan yang tidak memodifikasi file secara langsung, tetapi berdampak pada mereka melalui perubahan API internal. . Anda mungkin juga menemukan bahwa "penguncian pesimistis" semacam ini membuat perubahan di seluruh sistem dan refactoring yang dalam cukup merepotkan.

_{Sebagai perbandingan, tidak akan ada masalah seperti itu dalam ulasan pasca-komit (karena kesepakatan ini dengan kode yang sudah digabungkan menurut definisi) - tetapi karena Anda menargetkan ulasan pra-komit, ini mungkin tidak menarik.}

3. memuat pengembang yang sedang menunggu peninjauan

Menetapkan kebijakan eksplisit untuk apakah pengembang yang mengirimkan ulasan harus beralih ke tugas baru atau melakukan sesuatu yang lain (seperti misalnya mengejar resensi).

_{Sebagai perbandingan, ulasan pasca-komitmen hampir tidak memerlukan kebijakan eksplisit (karena itu wajar untuk melanjutkan ke tugas berikutnya setelah Anda melakukan kode dan mempertimbangkan tenggat waktu ulasan adalah satu atau dua minggu) - tetapi karena Anda menargetkan ulasan pra-komitmen, ini mungkin tidak menarik.}

Setiap bagian dari pengembangan yang perlu ditinjau harus di cabang terpisah. Jadi, cabang harus sudah ada sebelum waktunya datang untuk ditinjau. Maka langkah itu perlu:

1. Ulasan
2. Merevisi (mungkin)
3. kembali ke Ulasan (mungkin)
4. Gabungkan ke dalam trunk

Penggabungan adalah bagian yang sulit. Semakin lama cabang tetap independen, semakin sulit untuk menggabungkannya kembali ke bagasi. (Mungkin juga lebih sulit untuk menguji.)

Penggabungan silang adalah solusi yang memungkinkan. Sebelum menggabungkan ke dalam trunk (langkah 4, atau bahkan lebih awal, katakan sebelum langkah 3 atau langkah 1), gabungkan trunk ke cabang. Pengembang dapat melakukannya dan mengujinya. Kemudian cabang menangkap dengan batang dan menjadi lebih mudah untuk menggabungkannya ke batang. Penggabungan ke dalam bagasi paling baik dilakukan oleh Anda, atau siapa pun yang bertanggung jawab atas bagasi tersebut.

Beberapa orang mencoba rebase bukannya menggabungkan. Beberapa orang berpendapat bahwa rebase itu jahat. Itu adalah debat lain.