Apa sajakah praktik yang baik sebelum memeriksa kode sumber? [Tutup]

Tim saya menggunakan Team Foundation Server untuk kontrol sumber, dan hari ini saya memperbaiki beberapa aplikasi pengujian bug dan asap sebelum saya memeriksanya, tetapi saya lupa mengomentari beberapa kode. (Kode ini membuat UI sedikit aneh.)

Saya ingin tahu praktik apa yang ada sebelum memeriksa kode - Saya tidak ingin membuat kesalahan seperti ini lagi.

Satu hal yang biasa saya lakukan adalah selalu melihat perbedaan dari setiap file yang akan saya periksa, tepat sebelum saya memeriksanya.

Anda tidak boleh check-in kode komentar. Jika Anda memiliki kode yang perlu dikomentari sebelum check-in, Anda salah melakukannya.

Adapun aturan:

1. Dapatkan yang terbaru
2. Perbaiki konflik penggabungan

3. Membangun

   3.1 Memperbaiki kesalahan build

4. Jalankan tes

   4.1 Memperbaiki tes yang rusak

5. Pergi ke 1 (sampai tidak ada yang baru untuk didapatkan)

Hanya masuk saat semua langkah selesai.

Lihat tarian check-in .

Praktik baik lainnya:

• Tinjau daftar file yang akan diperiksa untuk memastikan mereka adalah file yang diharapkan.
• Tinjau perubahan untuk setiap file (penghapusan / penambahan / perbedaan)

Saya tidak mencoba untuk menjadi terlalu banyak di sini, tetapi asumsi dalam pertanyaan ini (dan semua kecuali salah satu jawaban) sebagian besar berlaku untuk VCS Terpusat, seperti TFS, SVN, Perforce, dll.
Cukup adil, itulah yang OP menggunakan.

Namun, di sisi lain, ketika menggunakan DVCS (seperti Mercurial dan Git), Anda biasanya tidak perlu menunggu untuk checkin, dan sebagian besar hal yang disebutkan dalam jawaban - seperti diff, dapatkan yang terbaru, gabungkan, dll - tidak diperlukan . Bahkan hal-hal seperti ulasan kode dan tes lebih baik dilakukan setelah checkin (meskipun mungkin sebelum mendorong, tergantung ...)
Satu-satunya pengecualian yang saya lihat di sini (sejauh ini) terkait dengan item pekerjaan. Tentu saja, mengomentari checkin juga bagus ...

Tiga hal yang tidak saya lihat dalam jawaban lain:

Sertakan file baru

• Cari file baru yang bukan bagian dari daftar perubahan Anda
• Mungkin spesifik untuk SCM seperti Perforce - Anda harus mengatakan semua yang ada di perubahan Anda.

Kembalikan file yang tidak berubah

• Saya benci ketika saya melihat perubahan orang lain dan ada daftar perubahan dengan sembilan file tetapi hanya tiga dari mereka yang telah dimodifikasi.
• Saya juga mengembalikan file dengan spasi putih atau perubahan yang tidak berarti.

Periksa komit yang Anda kirim

• Pastikan build tetap berwarna hijau setelah komit Anda.
• Saya dulu memiliki mesin kedua yang akan saya sinkronkan, bangun, dan jalankan setelah komit saya jadi jika ada masalah, saya dapat dengan mudah melompat dan memperbaikinya.

Dua hal ketika saya menggunakan Git:

Atom melakukan:

• Hanya tahap perubahan fungsional individu untuk komit.
• Buat komitmen sekecil mungkin. Buat mereka mudah ditambal, dikembalikan, dan dimengerti.
• Saya gunakan git add --patchuntuk membagi perubahan saya menjadi beberapa bagian jika perlu.

Lihat perbedaan saat meringkas

• Saya selalu memeriksa git commit --verbosesehingga saya dapat melihat perbedaan dari perubahan saya saat saya mengetikkan pesan komit saya. (Atau saya menggunakan git-vim tambalan saya untuk menampilkan diff.)
• Ini membuatnya lebih mudah untuk melalui perubahan Anda dan menggambarkan seluruh komit. Kadang-kadang, saya menangkap perubahan yang tidak diinginkan pada tahap ini. (Menjelaskan perubahan Anda akan membantu Anda memikirkannya.)

Beberapa item 'praktik bagus' yang saya terapkan di server tim saya cukup mudah. Pertama, sebelum Anda check-in, Anda harus selalu mendapatkan yang terbaru dan menjalankan build lokal, untuk memastikan bahwa tidak ada orang lain yang memeriksa apa pun yang kode Anda akan bentrok. Selain itu, atasi konflik kode apa pun di mesin lokal Anda, bukan server. Setelah kode Anda, dengan kode terbaru diunduh, telah dikonfirmasi untuk membangun dan berfungsi dengan baik Anda siap untuk langkah selanjutnya. Jalankan tes otomatis apa pun kemudian mulai check-in Anda untuk memastikan tes tersebut masih berfungsi dengan baik. Kemudian, hanya untuk memastikan, dapatkan yang terbaru lagi.

Mungkin, sebagai Admin TFS, untuk memberlakukan komentar pada semua check-in. Saya akan merekomendasikan selalu memasukkan komentar check-in untuk pekerjaan Anda terlepas dari apakah itu diberlakukan atau tidak. Jika Anda memiliki pilihan untuk melakukannya, tegakkan itu. Pastikan komentarnya, setidaknya, adalah ringkasan umum tentang apa yang Anda ubah sejak terakhir kali Anda memeriksa kode. Dengan begitu, jika terjadi kesalahan, Anda dapat melihat melalui check-in dan melihat, secara kasar, apa yang diubah pada saat check-in itu. Itu membuat debug bangunan yang rusak jauh lebih mudah.

Selain itu, jika Anda memiliki hak istimewa TFS Admin, memberlakukan rolling build berdasarkan check-in (untuk memastikan semua orang langsung tahu jika check-in mereka merusak sesuatu), dan Anda dapat mengatur server untuk melakukan check-in yang terjaga keamanannya ( jika kode yang diperiksa merusak build, server menolaknya), atau Anda dapat membuatnya membuat bug dan memberikannya kepada siapa pun yang melanggar build.

Ada beberapa opsi lain yang bisa Anda nyalakan atau matikan untuk menjaga agar semuanya tetap baik, atau untuk menyarankan kepada TFS-Admin Anda untuk menghidupkan agar barang-barang tetap bagus dan bersih ... tetapi mereka sebagian besar lebih disukai

Cari dan ganti kata-kata kutukan ;-)

Jika Anda check in dari Windows, periksa apakah kode Anda tidak memiliki karakter ^ M - editor di UNIX yang sering memberikan kesalahan / peringatan karena itu.

Juga coba dan ganti tab - pengguna yang berbeda akhirnya akan melihat tabstop berbeda beberapa dengan 4 spasi, beberapa 8 dan tidak baik untuk keterbacaan kode.

IMHO pendekatan terbaik adalah memiliki skrip yang telah ditentukan memeriksa kode Anda terhadap pedoman pengkodean organisasi Anda. Banyak sistem kontrol sumber memiliki fungsi ini.

Di perusahaan saya, kami menggunakan ulasan check-in. Ini tidak harus dirinci, tetapi hanya menunjukkan kepada seseorang perbedaan dalam daftar perubahan Anda dan berbicara melalui mereka kadang-kadang akan menyoroti kesalahan ketik aneh yang Anda lewatkan dalam pengujian.

Server kontrol sumber kami tidak akan mengizinkan Anda untuk check-in kecuali jika Anda mencatat nama pengulas di komentar (dalam formulir! Inisial, misalnya! BW jika Bruce Wayne melakukan review Anda). Peninjau mendapat email yang menyatakan bahwa mereka membantu meninjau. Ini terbuka untuk eksploitasi yang jelas tetapi tampaknya bekerja dengan cukup baik.

Kapan pun memungkinkan, saya ingin mengaitkan check-in dengan item pekerjaan. Ini memberi Anda beberapa informasi kontekstual tentang MENGAPA ini diubah, bukan hanya APA yang diubah. TFS memiliki sistem pelacakan item pekerjaan yang cukup baik, jadi ini agak sepele untuk dilakukan pada saat check-in.

(ini selain meninjau perbedaan perubahan saya)

Satu hal kecil yang saya lakukan adalah tidak memeriksa file yang belum benar-benar berubah. File-file ini mungkin telah dimodifikasi secara tidak sengaja, atau mungkin telah terlibat dalam refactor yang telah dibatalkan atau telah diperdebatkan.

Dengan cara ini, changeset Anda (terkait dengan item kerja) berisi persis file yang diperlukan untuk memenuhi item kerja.

Untuk menggabungkan semua jawaban di sini dan berikan daftar periksa lengkap

1. [check-in / check-out] Anda tidak boleh check-in langsung ke aliran tempat orang lain bekerja. Anda harus memiliki strategi aliran: misalnya per pengembang aliran di mana Anda dapat memeriksa dan memeriksa secara mandiri tanpa mengganggu orang lain: pekerjaan Anda akan aman tetapi dalam arus pengembangan Anda sendiri sehingga [hanya dalam aliran pengembangan Anda sendiri]. Dengan setiap cek di Anda kaitkan dengan catatan perubahan sehingga perubahan Anda adalah atom terhadap perubahan yang disebut set perubahan (sehingga Anda dapat mendistribusikan rfc individu / bug dll ... tanpa harus memberikan 'segalanya').

2. [lalu rebase dengan aliran tim Anda] itu berarti Anda mendapatkan perubahan dari orang lain di aliran Anda sendiri. Selama operasi itu, Anda dapat melihat dalam dialog menggabungkan semua "diff" dan pergi melalui mereka atau ... jika ada ribuan dan / atau Anda menggunakan bukan kode tetapi juga misalnya model data / proyek siebel dll ... bergantung pada salah satu non trivial merge, trivial-automatic dan trivial manual merge, kategori terakhir berisi yang sulit. Ingatlah bahwa Anda masih bekerja di aliran Anda sendiri.

3. [selesai rebase] Jika semuanya baik-baik saja maka periksa semua perubahan yang baru saja Anda dapatkan dari aliran tim: aliran Anda sendiri sekarang terbarui

4. [serahkan] sekarang serahkan pekerjaan Anda ke aliran tim. JIKA Anda tidak ingin memberikan semua yang Anda juga dapat memilih misalnya 1 RFC tertentu dengan versi file tertentu atau satu set RFC / cacat diselesaikan.

5. [test deliver] seharusnya tidak apa-apa, tetapi karena ada kemungkinan seseorang pada saat itu menyampaikan perubahan juga: Anda dapat menguji apakah pekerjaan Anda bekerja dengan perubahan terbaru pada aliran tim. Dengan dialog gabungan yang sama menunjukkan perbedaan.

6. [pengiriman lengkap] selesaikan pengiriman Anda dan pekerjaan Anda sekarang ada di aliran tim.

Untuk membuatnya lebih kompleks: Karena masih ada kemungkinan bahwa pekerjaan yang Anda kirimkan = ok TAPI Anda sudah bekerja pada versi lebih lanjut, Anda harus membuat garis dasar setelah memberikan dan menunjukkan garis dasar yang mana yang lebih disukai pengguna lain untuk diubah dari . Itu memastikan bahwa pengembang lain mendapatkan yang direkomendasikan dan bukan versi terbaru di aliran (jika Anda bekerja dalam skenario itu). Itu juga merupakan Triple check sehingga JIKA bahkan versi terbaru dalam aliran tim adalah "buruk" mereka masih bukan yang orang lain ulangi atau lihat dan manajer konfigurasi Anda dapat menggabungkan versi sebelumnya ke versi berikutnya untuk membatalkan pengiriman Anda.

• jawaban dari histumness terjadi 2 kali: pada langkah 2 dan 6
• jawaban dari Oded pada saat check-in dance: idem tetapi lapisan tambahan dari pengiriman dan rebase pada saat check-in / check out untuk memastikan Anda bekerja terisolasi dan kesalahan selalu dapat dengan mudah dihapus bahkan pada tahap selanjutnya
• jawaban dari guildsbounty yang dijawab: dapatkan yang terbaru adalah langkah 2. Untuk build: itu sangat tergantung jika Anda MEMILIKI build ... di dunia saya, Anda memiliki input dari model data, dokumen kata, lembar persyaratan, data konfigurasi dari informatica, siebel, dll .., dan ya juga kode java, .net code dll ... yang semuanya harus berbaur bersama. Jadi tidak ada yang benar-benar "membangun" di sini tetapi lebih tinggi integrasi tergantung apakah itu misalnya membangun dari "kode" Anda terintegrasi dengan semua hal-hal lain karena Anda tidak bisa tahu pasti apakah itu barang integrasi dan tergantung pada lingkungan pengujian mereka itu bisa dikompilasi barang diperlukan atau pada pengiriman yang lebih tinggi membangun lain karena itu membutuhkan sesuatu dari tim lain.
• jawaban dari guildsbounty pada komentar dan diperlukan: saya pikir setiap lingkungan di mana Anda tidak memiliki integrasi VERSI dan Perubahan dalam set perubahan (dan ketik: cacat, RFC, hotfi) tidak matang. Saya pikir itu kekacauan jika Anda tidak dapat misalnya mengotomatiskan catatan rilis dengan jumlah bug dan rfc yang dikirimkan yang dapat diklik melalui ke versi yang tepat yang disentuh (karena misalnya versi 1 dan versi 3 dari hello.c bisa sangat baik disampaikan tetapi versi 2 seharusnya tidak dikirim karena barang-barang di sana akan menjadi bagian dari rilis nanti tetapi beberapa noob sudah memasukkannya) (jadi itu berarti keputusan manual JIKA Anda juga ingin mengeluarkan versi 3 dari halo. c TETAPI mengeluarkan versi 3 berarti Anda juga harus menghapus semua versi lain yang disentuh oleh RFC / cacat tersebut sehingga Anda harus dapat dengan mudah dan cepat dengan alat untuk mengeluarkan semuanya secara lengkap) (bahkan jika banyak pengembang mengerjakan bagian dari RFC yang sama) tetapi setidaknya Anda membutuhkan barang-barang di sekitarnya untuk memutuskannya dll ...). Dokumentasi tambahan selalu berguna tetapi dengan mengaitkan set perubahan Anda mendapatkan lingkaran penuh: versi <- set perubahan <- item kerja <- tiket / rfc / cacat <- persyaratan. Artinya: Anda tahu persyaratan mana yang sepenuhnya atau sepenuhnya disampaikan: satu persyaratan memiliki beberapa RFC atau bug atau apa pun. RFC memiliki banyak item pekerjaan untuk beberapa orang. bahwa item pekerjaan sesuai dengan set perubahan yang ada dari set versi (misalnya versi 1 dan 3 dari hello.c pada aliran integrasi yang tentu saja BUKAN versi 1,
• komentar dari luis.espinal: dont break the build dicek ulang dalam rebase dan tetap dikirimkan ... ada pengiriman yang lebih tinggi untuk 'manajer pelepasliaran dan pembangun' yang harus melihat set perubahan dan data dasar sebagai info mereka. "Tidak pernah bekerja secara langsung di cabang utama" ya, struktur aliran bisa besar atau sederhana tetapi pada dasarnya: pengembang memiliki aliran mereka sendiri, mereka mengirimkan ke aliran tim yang mengirim ke aliran rilis. -> sehingga pengiriman dari semua tim (misalnya tim dokumentasi, tim persyaratan, tim pengembangan,

Dalam contoh Anda, Anda memberi Anda lupa berkomentar kode. Kesalahan terjadi. Sistem manajemen konfigurasi di sekitarnya harus mengatasinya. Bisa jadi satu misalnya ribuan perubahan datang dan "membangun" dan "integrasi" terjadi dalam hierarki aliran pada server yang berbeda dirantai dan diproses dalam waktu. Jadi, bahkan jika setelah 5 bulan kode komentar Anda diuji pada server integrasi karena kode Anda membutuhkan integrasi dengan kode dan sistem lain, masih mungkin untuk secara atomik mengambil set perubahan Anda dan masih melanjutkan. Jadi praktik terbaik kurang lebih pada tingkat yang lebih tinggi. Desain keseluruhan aliran manajemen konfigurasi harus mengatasinya. Untuk pengembang individu praktik terbaik adalah tentu saja untuk memvalidasi / uji unit. Tapi dari gambaran besar sampai "

Beberapa dari yang berikut ini berlaku lebih dari yang lain (atau dalam bentuk yang berbeda) tergantung pada SCM Anda, jadi inilah mereka:

1. Jangan merusak build - periksa hanya kode yang mengkompilasi.
2. Komentari lapor masuk Anda (dan mungkin lapor keluar Anda jika SCM memberi Anda opsi itu.)
3. Jangan biarkan barang-barang tidak dicentang untuk jangka waktu yang lama.
4. Sering-seringlah check-in (beberapa kali sehari).
5. Beri label secara bermakna.
6. Beri label secara teratur.
7. Jangan pernah bekerja langsung di cabang utama.
8. Setiap rilis untuk produksi harus memiliki label sendiri (dan cabang read-only dari cabang utama jika memungkinkan). Jika memungkinkan, lakukan hal yang sama untuk rilis uji UAT / Integrasi / Pra-produksi.
9. Anda harus dapat membangun apa yang ada di produksi dari apa yang ada di SCM Anda dan dari label.

CATATAN : beberapa item di atas tampak agak jelas, tetapi Anda tidak akan percaya berapa banyak orang yang benar-benar bekerja di cabang utama atau membuat perubahan pada produksi pertama dan kemudian secara manual membuat delta untuk pergi pada kontrol versi ... langsung di cabang utama. .. dan dengan label. Manis seperti empedu fermentasi dicampur dengan jus ketiak yang tidak dicuci ... ya, seperti itu.

Memiliki daftar periksa pribadi. Mulai kosong saat Anda mengacaukan, pada sebuah entri. Ketika menjadi sifat kedua hapus dari daftar.

Jalankan tes. Jika mereka lulus check in. Jika Anda mengacaukan dan sesuatu melewati tes, kemudian menulis tes.

Kami melakukan yang berikut ...

1. Tes - Kami ingin memastikan itu berfungsi. Paling tidak, kami ingin tahu bahwa itu tidak merusak apa pun.

2. Peninjauan kode, atau setidaknya pemeriksaan teman - Ini adalah cara yang bagus untuk memastikan bahwa pengetahuan tersebar dan orang-orang selalu terbarui. Ini juga membantu menangkap bug sebelum memeriksa.

3. Kirim pemberitahuan sebelumnya - Pemberitahuan sebelumnya dikirim ke grup sebelum checkin. Tujuannya tidak hanya untuk memberi tahu orang lain file atau area mana yang berubah, tetapi juga memberi mereka informasi (jika mereka memilih untuk memperhatikan) jika perubahan itu diharapkan mempengaruhi mereka.

4. Beberapa jam setelah mengirim pemberitahuan sebelumnya, check-in dilakukan, dan grup diinformasikan melalui email. Setiap orang dalam grup dapat mengetahui kapan fitur bug atau fitur tertentu selesai.

5. Salinan pemberitahuan checkin disisipkan ke dalam catatan perbaikan yang terkait dengan bug atau fitur. Saat mencari di seluruh catatan, kami menemukan bahwa sangat praktis untuk mengetahui apa yang termasuk dalam perbaikan / fitur.

Jalankan tes unit yang telah Anda garap dengan rajin. Hijau itu baik.

Pastikan kode Anda diformat dengan benar (misalnya untuk Java: pilih kode Anda dan tekan Ctrl-Shift-F di Eclipse). Tapi hati-hati melakukan hal yang sama untuk seluruh dokumen.

Selalu, selalu, selalu , periksa bahwa perubahan apa pun yang Anda lakukan tidak merusak build. Terutama perubahan kecil yang mungkin tampak sepele.

Suatu kali saya membuat perubahan kecil yang saya pikir tidak akan menimbulkan masalah tepat sebelum saya meninggalkan pekerjaan untuk akhir pekan. Benar saja, perubahan kecil itu merusak pembangunan dan tidak ada uji coba malam dilaksanakan untuk proyek kami. Kepala Q&A tidak terlalu senang tentang itu, dan memang begitu.

Cari bagian dari perubahan Anda yang bisa masuk sebagai unit mandiri.

Seringkali, pada saat saya memiliki perbaikan atau penyempurnaan kode, ada beberapa perubahan. Beberapa dari mereka khusus untuk perubahan perilaku yang saya jalani; yang lain adalah refactoring yang saya lakukan untuk membuat perubahan itu lebih bersih.

Saya lebih suka memeriksa di setiap refactoring secara terpisah, dengan deskripsi perubahannya sendiri, seperti ini:

REFACTORING: Ganti nama X menjadi Y

X masuk akal sebelumnya karena ... tapi sekarang harus Y. Ini terkait dengan pekerjaan untuk masalah # 9.

Kemudian, setelah setiap refactoring yang baik diperiksa, perubahan perilaku akhir sering sepele.

Juga, beberapa perubahan memengaruhi banyak baris kode tetapi tidak terlalu menarik, sementara perubahan lainnya sangat lokal tetapi memiliki dampak penting. Jika perubahan ini dicentangkan bersama, mungkin sulit untuk membaca perbedaan. Jadi, saya memisahkan mereka.

Kemudian, ketika seseorang membaca sejarah perubahan, jelaslah bagaimana segala sesuatunya sampai pada keadaan saat ini, dan mengapa hal itu terjadi. Juga sepele untuk membatalkan perubahan perilaku saya karena tidak kusut dengan banyak pengeditan lainnya.

Lakukan apa yang akan Anda lakukan ketika mengembalikan sesuatu yang Anda pinjam dari seseorang. Pastikan itu bersih dan dalam kondisi baik. Jika Anda melakukan kekacauan, pastikan untuk membersihkan sebelum mengembalikan kode ke pemiliknya (dalam kebanyakan kasus, majikan Anda).

Saya menyimpan repo hg lokal untuk pekerjaan saya.

• Setiap kali saya memeriksa sesuatu, saya mencari di diff dan memastikan bahwa semua perubahan dapat diterima.
• Saya mencoba mencatat fitur utama checkin.
• Saya mencoba untuk menjaga setiap ukuran komit ke satu fitur utama.

Saya tidak mengklaim ini adalah yang terbaik, tetapi mereka bekerja untuk saya.

Ketika saya menulis kode yang saya tahu tidak dimaksudkan untuk diperiksa, saya menambahkan baris sebelum berisi "// TEMP:" dan setelahnya dengan "// END TEMP.". Ini, bersama dengan melakukan diff sebelum check-in, berjanji bahwa saya tidak akan memeriksa kode itu secara tidak sengaja.

Tes secara menyeluruh semua yang Anda tambahkan atau ubah. Cobalah setiap kasus yang mungkin Anda bisa pikirkan untuk mencoba. Jangan tinggalkan pengujian ke QA. Jika saya memiliki sandwich untuk setiap kali saya membuat beberapa perubahan kecil, dan kemudian mencoba beberapa test case hanya untuk berada di sisi yang aman, dan segera melihat masalah, saya akan memiliki banyak sandwich. Saya biasanya berkata dengan keras kepada diri sendiri, "Saya sangat senang saya mencoba itu ..."

Anda mengatakan UI menjadi aneh setelah perubahan Anda. Jika Anda hanya menjalankan program dan melihat UI sebelum check in, apakah Anda akan memperhatikan masalahnya?