Cara membuat ulasan kode lebih cepat dan lebih efisien

Bagaimana biasanya tinjauan kode terjadi? Anda mengirim permintaan kumpulan, mendapatkan umpan balik, melakukan koreksi, mengirim perbaikan untuk ulasan kedua, kemudian mendapatkan persetujuan, dan menggabungkan terjadi. Kedengarannya sederhana, tetapi dalam kenyataannya proses peninjauan bisa sangat memakan waktu.

Bayangkan Anda memiliki permintaan kumpulan dengan ratusan garis perubahan. Peninjau harus menghabiskan banyak waktu untuk membaca kode sepenuhnya dan memahami perubahan yang diajukan. Akibatnya, seluruh proses mulai dari membuat permintaan kumpulan hingga persetujuannya bisa memakan waktu beberapa hari - ini tidak terlalu menyenangkan bagi pengkaji dan penulis perubahan. Dan kemungkinannya bagus bahwa pada akhirnya reviewer akan tetap melewatkan sesuatu. Atau cek mungkin terlalu dangkal, dan dalam kasus terburuk, permintaan kumpulan dapat segera ditolak.

Ternyata semakin besar permintaan pool, semakin sedikit manfaat dari memeriksanya.

Bagaimana cara menghindari situasi seperti itu? Bagaimana cara membuat permintaan kumpulan lebih mudah dan lebih mudah dipahami oleh pengkaji dan mengoptimalkan seluruh proses?

Kami menerjemahkan sebuah artikel oleh pengembang backend kami Sergey Zhuk tentang bagaimana proses peninjauan kode dari tim aplikasi seluler Skyeng bekerja.

Ubah Kategori

Mari kita bayangkan bahwa Anda memiliki tugas - untuk mengimplementasikan fungsionalitas baru dalam proyek. Permintaan kumpulan yang sedang Anda kerjakan mungkin berisi berbagai kategori perubahan. Tentu saja akan ada beberapa kode baru di dalamnya. Tetapi dalam pekerjaan, Anda mungkin memperhatikan bahwa beberapa kode perlu di refactored sebelumnya sehingga berkontribusi pada penambahan fungsionalitas baru. Atau dengan fungsi baru ini, duplikasi muncul di kode yang ingin Anda hilangkan. Atau Anda tiba-tiba menemukan kesalahan dan ingin memperbaikinya. Seperti apa bentuk permintaan akhir kumpulan?

Pertama, mari kita lihat kategori perubahan apa yang dapat terjadi dengan kode.

1. Perubahan fungsional.
2. Refactoring struktural - perubahan kelas, antarmuka, metode, perpindahan antar kelas.
3. Refactoring sederhana - dapat dilakukan menggunakan IDE (misalnya, mengekstraksi variabel / metode / konstanta, menyederhanakan kondisi).
4. Mengganti nama dan memindahkan kelas - mengatur ulang namespace.
5. Menghapus kode yang tidak digunakan (mati).
6. Gaya kode koreksi.

Tinjauan Strategi

Sangat penting untuk memahami bahwa masing-masing kategori ditinjau secara berbeda dan ketika mempertimbangkannya, peninjau harus fokus pada hal-hal yang berbeda. Dapat dikatakan bahwa setiap kategori perubahan melibatkan strategi peninjauannya sendiri.

1. Perubahan fungsional: menyelesaikan masalah bisnis dan desain sistem.
2. Refactoring struktural: kompatibilitas ke belakang dan peningkatan desain.
3. Refactoring primitif: peningkatan keterbacaan. Perubahan-perubahan ini terutama dapat dilakukan dengan menggunakan IDE (misalnya, mengekstraksi variabel / metode / konstanta, dll.).
4. Mengganti nama / memindahkan kelas: apakah struktur namespace ditingkatkan?
5. Menghapus kode yang tidak digunakan: kompatibel ke belakang.
6. Gaya kode koreksi: paling sering menggabungkan permintaan kumpulan terjadi segera.

Pendekatan yang berbeda digunakan untuk memeriksa perubahan dalam kategori yang berbeda, oleh karena itu, jumlah waktu dan upaya yang dihabiskan untuk tinjauan mereka juga bervariasi.

Perubahan fungsional. Ini adalah proses terpanjang karena melibatkan perubahan logika domain. Peninjau mencari untuk melihat apakah masalah teratasi dan memeriksa apakah solusi yang diusulkan adalah yang paling cocok atau dapat diperbaiki.

Refactoring struktural. Proses ini membutuhkan waktu jauh lebih sedikit daripada perubahan fungsional. Tetapi mungkin ada saran dan ketidaksepakatan tentang bagaimana tepatnya kode harus diatur.

Saat memeriksa kategori yang tersisa di 99% kasus, penggabungan terjadi segera.

1. Refactoring sederhana. Apakah kode menjadi lebih mudah dibaca? - merzhim.
2. Mengganti nama / memindahkan kelas. Apakah kelas telah dipindahkan ke namespace yang lebih baik? - Merzh.
3. Menghapus kode yang tidak digunakan (mati) adalah merzhim.
4. Gaya atau pemformatan kode koreksi - merzh. Kolega Anda tidak boleh memeriksa ini selama peninjauan kode, ini adalah tugas dari si penulis.

Mengapa perubahan harus dikategorikan?

Kami telah membahas bahwa berbagai kategori perubahan direvisi secara berbeda. Misalnya, kami memverifikasi perubahan fungsional berdasarkan persyaratan bisnis, dan dalam refactoring struktural, kami memeriksa kompatibilitas ke belakang. Dan jika kita mencampur beberapa kategori, akan sulit bagi peninjau untuk mengingat beberapa strategi peninjauan dalam waktu yang bersamaan. Dan, kemungkinan besar, pengulas akan menghabiskan lebih banyak waktu pada permintaan kolam daripada yang diperlukan, dan karena ini, ia mungkin kehilangan sesuatu. Selain itu, jika permintaan kumpulan berisi berbagai jenis perubahan, dengan koreksi apa pun pengulas harus merevisi semua kategori ini lagi. Misalnya, Anda mencampur perubahan struktural dan perubahan fungsional. Sekalipun refactoring dilakukan dengan baik, tetapi ada masalah dengan implementasi fungsional, maka setelah koreksi, reviewer harus melihat seluruh permintaan pool sejak awal. Yaitu, periksa kembali perubahan refactoring dan fungsional. Jadi pengulas menghabiskan lebih banyak waktu atas permintaan kolam renang. Alih-alih segera merenungkan permintaan kumpulan terpisah dengan refactoring, peninjau harus sekali lagi meninjau seluruh kode.

Apa sebenarnya tidak layak dicampurkan

Mengganti nama / menghapus kelas dan refactoring-nya. Di sini kita menemukan Git, yang tidak selalu benar memahami perubahan tersebut. Maksud saya perubahan skala besar ketika banyak garis berubah. Ketika Anda mereformasi sebuah kelas dan kemudian memindahkannya ke suatu tempat, Git tidak menganggapnya sebagai bergerak. Sebaliknya, Git mengartikan perubahan ini sebagai menghapus satu kelas dan membuat yang lain. Ini mengarah ke banyak pertanyaan selama tinjauan kode. Dan penulis kode ditanya mengapa ia menulis kode yang jelek ini, meskipun sebenarnya kode ini hanya dipindahkan dari satu tempat ke tempat lain dengan perubahan kecil.

Setiap perubahan fungsional + refactoring apa pun. Kami sudah membahas kasus ini di atas. Hal ini membuat pengulas mengingat dua strategi ulasan sekaligus. Bahkan jika refactoring dilakukan dengan baik, kami tidak akan dapat menunda perubahan ini sampai perubahan fungsional disetujui.

Setiap perubahan mekanis + setiap perubahan yang dilakukan oleh manusia. Yang dimaksud dengan "perubahan mekanis" adalah pemformatan yang dilakukan menggunakan IDE atau pembuatan kode. Misalnya, kami menerapkan gaya kode baru dan mendapatkan 3000 perubahan baris. Dan jika kita mencampurkan perubahan ini dengan perubahan fungsional atau perubahan lain apa pun yang dilakukan oleh seseorang, kami akan memaksa pengulas untuk secara mental mengklasifikasikan perubahan dan alasan ini: ini adalah perubahan yang dilakukan oleh komputer - itu dapat dilewati, dan perubahan yang dilakukan oleh seseorang diperlukan lihat. Jadi reviewer menghabiskan banyak waktu untuk memeriksa.

Contoh

Berikut ini adalah permintaan kumpulan dengan fungsi metode yang dengan lembut menutup koneksi klien ke Memcached:



Sekalipun permintaan kumpulan kecil dan berisi hanya seratus baris kode, masih sulit untuk memverifikasi. Seperti yang dapat Anda lihat dari komitmen, ini berisi berbagai kategori perubahan:

• fungsional (kode baru),
• refactoring (membuat / memindahkan kelas),
• gaya kode koreksi (penghapusan kelebihan blok dermaga).

Pada saat yang sama, fungsi baru itu sendiri hanya 10 baris:



Akibatnya, peninjau harus meninjau seluruh kode dan

• Verifikasi bahwa refactoring tidak masalah.
• periksa apakah fungsionalitas baru diterapkan dengan benar;
• tentukan apakah perubahan ini secara otomatis dihasilkan oleh IDE atau oleh orang tersebut.

Karenanya, permintaan kumpulan seperti itu sulit untuk ditinjau. Untuk memperbaiki situasi, Anda dapat memecah komit ini menjadi cabang terpisah dan membuat kumpulan permintaan untuk masing-masing.

1. Refactoring: ekstraksi kelas

Hanya ada dua file. Peninjau hanya harus memeriksa desain baru. Jika semuanya beres - merzhim.

2. Langkah selanjutnya adalah refactoring, kami hanya memindahkan dua kelas ke namespace baru

Permintaan kumpulan seperti itu cukup sederhana untuk diperiksa, bisa langsung dipakai.

3. Menghapus kelebihan blok dermaga

Tidak ada yang menarik di sini. Merzhim.

4. Fungsional itu sendiri

Dan sekarang permintaan kumpulan dengan perubahan fungsional hanya berisi kode yang diinginkan. Jadi resensi Anda hanya bisa fokus pada tugas ini. Permintaan kolam kecil dan mudah diperiksa.

Kesimpulan

Aturan praktis:

Jangan membuat permintaan kumpulan besar dengan kategori perubahan campuran.

Semakin besar permintaan kumpulan, semakin sulit bagi peninjau untuk memahami perubahan yang Anda usulkan. Kemungkinan besar, permintaan kumpulan besar dengan ratusan baris kode akan ditolak. Sebagai gantinya, pecah menjadi beberapa bagian logis kecil. Jika refactoring Anda berurutan, tetapi perubahan fungsional mengandung kesalahan, maka refactoring dapat dengan mudah ditahan, sehingga Anda dan pengulas Anda dapat fokus pada fungsionalitas tanpa melihat semua kode sejak awal.

Dan selalu ikuti langkah-langkah ini sebelum mengirim permintaan kumpulan:

• Optimalkan kode Anda untuk membaca. Kode jauh lebih mudah dibaca daripada ditulis;
• Jelaskan perubahan yang diajukan untuk memberikan konteks yang diperlukan untuk pemahaman mereka;
• selalu periksa kode Anda sebelum membuat permintaan kumpulan. Dan lakukan seolah-olah itu adalah kode orang lain. Terkadang membantu menemukan sesuatu yang Anda lewatkan. Ini akan mengurangi kemungkinan penolakan terhadap permintaan kumpulan Anda dan jumlah koreksi.

Rekan saya Oleg Antonevich memberi tahu saya tentang ide untuk memecah perubahan ke dalam kategori.

Dari Editor: Sergey menulis banyak hal menarik tentang pemrograman dan PHP, dan kadang-kadang kita menerjemahkan sesuatu: server video streaming , merender file HTML . Jangan ragu untuk bertanya kepadanya di komentar untuk artikel ini - dia akan menjawab!

Yah, kami juga mengingatkan Anda bahwa kami selalu memiliki banyak lowongan menarik untuk pengembang!