Geekly articles weekly

Ulasan Kode: Pengalaman Sukses



Ada banyak informasi di Internet tentang ulasan kode:

  • bagaimana peninjauan kode orang lain memengaruhi budaya perusahaan
  • Pemeriksaan Keamanan Regulasi
  • manual ringkas
  • daftar panjang rekomendasi
  • review kode dari perspektif interpersonal
  • mengapa saya perlu tinjauan kode
  • metode yang terbukti
  • lebih lanjut tentang metode yang terbukti
  • statistik: seberapa banyak ulasan kode membantu menangkap bug
  • contoh kesalahan yang dibuat selama tinjauan kode

Ya, tentu saja, ada buku tentang hal ini. Singkatnya, artikel ini menjelaskan bagaimana tinjauan kode diatur oleh Palantir. Dalam organisasi-organisasi yang budayanya tidak menerima tinjauan sejawat semacam itu, mungkin berguna untuk pertama-tama membaca esai brilian Karl E. Wiegers, The Code Revision with a Human Face, dan kemudian mencoba mengikuti panduan ini.

Teks ini diambil dari rekomendasi peningkatan kualitas berdasarkan bekerja dengan Baseline , alat kami untuk kontrol kualitas kode Java. Ini mencakup topik-topik berikut:

  • Mengapa, apa, dan kapan kita mencoba mencapai tinjauan kode
  • Mempersiapkan kode untuk ditinjau
  • Eksekusi tinjauan kode
  • Contoh Tinjauan Kode

Diterjemahkan ke Alconost


Komik XKCD 'Kualitas Kode', disalin di bawah lisensi CC BY-NC 2.5

Motivasi


Kami terlibat dalam tinjauan kode (RC) untuk meningkatkan kualitas kode dan dengan demikian ingin secara positif mempengaruhi tim dan budaya perusahaan. Sebagai contoh:

  • Penulis kode termotivasi karena mereka tahu bahwa tim peninjau mereka akan mempertimbangkan permintaan mereka untuk perubahan: penulis mencoba untuk membersihkan kekasaran, mengikuti rencana aksi dan umumnya meningkatkan seluruh komitmen. Ketika kolega mengakui profesionalisme Anda dalam menulis kode - bagi banyak programmer ini adalah kesempatan untuk bangga dengan diri mereka sendiri.
  • Berbagi pengetahuan membantu tim pengembangan dalam beberapa cara:

- Di bawah RK, secara jelas dilaporkan bagian mana dari fungsi yang telah ditambahkan / diubah / dihapus sehingga di masa depan anggota tim dapat mengandalkan pekerjaan yang sudah dilakukan.

- Penulis kode dapat menggunakan teknik atau algoritma yang pengulas sendiri akan mempelajari sesuatu. Secara keseluruhan, tinjauan kode membantu meningkatkan standar kualitas di seluruh organisasi .

- Peninjau dapat mengetahui sesuatu tentang teknik pemrograman atau basis kode itu sendiri yang akan membantu meningkatkan atau mengkonsolidasikan perubahan yang dibuat; misalnya, seseorang dapat sekaligus mengembangkan atau memodifikasi peluang yang sama persis.

- Kontak dan komunikasi positif memperkuat ikatan sosial antara anggota tim.

  • Karena konsistensi dalam basis kode, kode itu sendiri jauh lebih mudah dibaca dan dipahami. Lebih mudah untuk mencegah bug dan mempromosikan kolaborasi antara programmer yang menetap dan nomaden.
  • Sulit bagi pembuat kode untuk menilai keterbacaan ciptaannya sendiri, dan bagi peninjau itu mudah, karena ia tidak melihat dalam konteks apa kode ini berada. Kode yang dapat dibaca manusia lebih mudah untuk digunakan kembali, memiliki lebih sedikit bug, dan lebih tahan lama.
  • Kesalahan acak (mis. Kesalahan pengetikan), serta kesalahan struktural (mis. Kode tidak dapat dieksekusi, kesalahan dalam logika atau algoritme, masalah kinerja dan arsitektur) sering kali lebih mudah ditangkap oleh pengulas pemilih yang melihat kode dari samping. Menurut penelitian, bahkan tinjauan kode pendek dan tidak resmi secara signifikan mempengaruhi kualitas kode dan terjadinya bug .
  • Persyaratan kepatuhan dan kerangka kerja peraturan seringkali membutuhkan tinjauan wajib. Tinjauan kode adalah cara terbaik untuk menghindari masalah keamanan umum. Jika persyaratan keamanan dalam lingkungan ini atau sehubungan dengan peluang ini meningkat, maka peninjauan akan direkomendasikan (atau bahkan diminta) oleh satraps dari otoritas pengawas (contoh yang baik adalah panduan OWASP ).

Apa yang harus dicari


Tidak ada jawaban yang jelas untuk pertanyaan ini, dan setiap tim pengembangan perlu menyepakati pendekatannya sendiri. Beberapa tim lebih suka memeriksa setiap perubahan yang dilakukan pada cabang pengembangan utama, sementara yang lain memperhitungkan "ambang batas triviality", di luar yang verifikasi wajib. Dalam hal ini, perlu untuk menyeimbangkan antara penggunaan waktu programmer secara efisien (baik penulis dan pengkaji kode) dan menjaga kualitas kode. Dalam beberapa konteks yang ketat, kadang-kadang diperlukan untuk memeriksa semuanya, bahkan perubahan paling sepele, dalam kode.

Aturan peninjauan kode adalah sama untuk semua orang: bahkan jika Anda adalah pengembang paling senior di tim, ini tidak berarti bahwa kode Anda tidak memerlukan peninjauan. Bahkan jika (kadang-kadang terjadi) kodenya sempurna, ulasan itu membuka peluang untuk bimbingan dan kerja sama, dan, setidaknya, membantu untuk melihat basis kode dari sudut pandang yang berbeda.

Kapan harus memeriksa


Peninjauan kode harus terjadi setelah berhasil menyelesaikan pemeriksaan otomatis (tes, desain, tahap integrasi berkelanjutan lainnya), tetapi sebelum penggabungan kode baru dengan cabang repositori utama. Biasanya kami tidak menggunakan verifikasi formal dari total perubahan yang dilakukan pada kode sejak rilis terakhir.

Untuk perubahan kompleks yang harus ditambahkan ke cabang utama dari kode sebagai satu unit, tetapi sangat luas sehingga mereka tidak dapat benar-benar tercakup dalam satu pemeriksaan, cobalah tinjauan bertahap. Kami membuat fitur utama / cabang fitur besar dan sejumlah yang sekunder (mis. Fitur / fitur besar-api, pengujian fitur / fitur besar, dll.), Yang masing-masing merangkum subset dari fungsi umum, dan masing-masing cabang tersebut diperiksa terhadap cabang utama fitur / fitur besar. Setelah semua cabang sekunder digabung menjadi fitur / fitur besar, buat tinjauan kode untuk menyuntikkan yang kedua ke cabang utama.

Mempersiapkan kode untuk ditinjau


Penulis kode wajib memberikan kode untuk ulasan dalam bentuk yang dapat dicerna agar tidak mengambil waktu ekstra dari pengulas dan tidak menurunkan motivasi mereka:

  • Lingkup dan ukuran . Perubahan harus terkait dengan ruang lingkup yang sempit, terdefinisi dengan baik, mandiri, sepenuhnya dicakup oleh tinjauan. Misalnya, perubahan mungkin terkait dengan penerapan beberapa fitur atau perbaikan bug. Perubahan singkat lebih disukai daripada perubahan panjang. Jika peninjauan berisi materi yang mencakup perubahan dalam lebih dari ~ 5 file, atau membutuhkan lebih dari 1-2 hari untuk merekam, atau peninjauan itu sendiri mungkin memakan waktu lebih dari 20 menit, cobalah memecah bahan menjadi beberapa fragmen yang berdiri sendiri dan memeriksa masing-masing secara terpisah. Misalnya, pengembang dapat mengirimkan perubahan yang mendefinisikan API untuk fitur baru dalam hal antarmuka dan dokumentasi, dan kemudian menambahkan perubahan kedua yang menjelaskan implementasi untuk antarmuka ini.
  • Anda hanya perlu mengirim materi yang lengkap, teruji secara independen (menggunakan diff) dan yang teruji secara independen . Untuk menghemat waktu peninjau, uji perubahan yang diajukan (mis., Jalankan rangkaian uji) dan pastikan bahwa kode melewati semua rakitan, serta semua tes dan semua tahap kontrol kualitas, baik secara lokal dan pada server integrasi berkelanjutan, dan hanya kemudian pilih pengulas
  • Perubahan refactoring seharusnya tidak mempengaruhi perilaku dan sebaliknya; perubahan yang memengaruhi perilaku tidak boleh melibatkan refactoring atau memformat ulang kode. Ada sejumlah alasan bagus untuk ini:

- Perubahan yang terkait dengan refactoring biasanya memengaruhi banyak baris kode di banyak file - oleh karena itu, kode ini tidak akan diperiksa dengan hati-hati . Perubahan perilaku yang tidak direncanakan dapat bocor ke basis kode, dan tidak ada yang akan menyadarinya.

- Perubahan besar yang terkait dengan refactoring melanggar mekanisme gerakan, seleksi dan "sihir" lainnya yang terkait dengan kontrol sumber. Sangat sulit untuk membatalkan perubahan perilaku yang dibuat setelah selesainya refactoring yang mencakup seluruh repositori.

- Waktu kerja mahal yang dihabiskan seseorang untuk meninjau kode harus pergi untuk memeriksa logika program, dan bukan pada perselisihan tentang gaya, sintaksis, atau pemformatan kode. Kami lebih suka menyelesaikan masalah seperti itu menggunakan alat otomatis, khususnya, Checkstyle , TSLint , Baseline , Prettier , dll.

Pesan komit


Berikut ini adalah contoh pesan komit yang kompeten, yang dirancang sesuai dengan standar yang banyak digunakan:

 ( 80 )    ,  ,   .   72 .          ,    —   .   ,       (   );     ,  rebase,    .  -   : « »,   « »   « ».      -,  , ,  git merge  git revert.      .    .

Cobalah untuk menggambarkan perubahan yang dilakukan selama komit, dan bagaimana tepatnya perubahan ini dibuat:

 > .   .    . > .  jcsv-     IntelliJ

Cari pengulas


Biasanya, penulis komit mencari satu atau dua pengulas yang akrab dengan basis kode. Seringkali dalam kapasitas ini adalah manajer proyek atau insinyur senior. Dianjurkan bagi pemilik proyek untuk berlangganan proyeknya sendiri untuk menerima pemberitahuan ulasan kode baru. Dengan partisipasi tiga atau lebih peninjau, peninjauan kode sering kali ternyata tidak produktif atau kontraproduktif, karena peninjau yang berbeda dapat mengusulkan perubahan yang saling bertentangan. Ini mungkin menunjukkan ketidaksepakatan mendasar tentang implementasi yang benar, dan masalah seperti itu tidak boleh diselesaikan selama tinjauan kode, tetapi pada pertemuan yang diperpanjang di mana semua pihak yang berkepentingan berpartisipasi secara langsung atau dalam mode konferensi video.

Eksekusi tinjauan kode


Peninjauan kode adalah titik sinkronisasi antara anggota tim yang berbeda, sehingga berpotensi menghentikan pekerjaan. Oleh karena itu, tinjauan kode harus operasional (memakan waktu beberapa jam, bukan hari), dan anggota tim dan pemimpin harus menyadari berapa banyak waktu yang diperlukan untuk memeriksa, dan memprioritaskan waktu yang dialokasikan untuk itu. Jika menurut Anda Anda tidak punya waktu untuk menyelesaikan ulasan tepat waktu, segera beri tahu penulis tentang hal itu sehingga ia dapat menemukan pengganti untuk Anda.

Tinjauan harus cukup menyeluruh sehingga peninjau dapat menjelaskan perubahan ke pengembang lain dengan detail yang cukup. Jadi kami akan memastikan bahwa detail basis kode diketahui setidaknya dua, dan bukan satu.

Anda, sebagai peninjau, harus mematuhi standar kualitas kode dan menjaganya pada tingkat tinggi. Tinjauan kode lebih merupakan seni daripada sains. Ini hanya bisa dipelajari dalam praktik. Peninjau yang berpengalaman harus mencoba dulu untuk membiarkan kolega yang kurang berpengalaman berubah dan membiarkan mereka menjadi yang pertama mengulas. Jika penulis mengikuti aturan di atas (terutama yang terkait dengan pengujian diri dan eksekusi kode awal), maka inilah yang harus diperhatikan oleh peninjau saat meninjau:

Tujuan


  • Apakah kode mencapai tujuan yang ditetapkan oleh penulis? Setiap perubahan harus memiliki alasan spesifik (fitur baru, refactoring, perbaikan bug, dll.). Apakah kode yang diajukan benar-benar membawa kita ke tujuan ini?
  • Ajukan pertanyaan. Fungsi dan kelas harus dibenarkan. Jika penugasan mereka kepada peninjau tidak jelas, mungkin ini berarti bahwa kode harus ditulis ulang atau didukung oleh komentar atau tes.

Implementasi


  • Pikirkan tentang bagaimana Anda akan menyelesaikan masalah ini. Jika tidak, lalu mengapa? Apakah kode Anda menangani lebih banyak kasus (batas)? Mungkin lebih pendek / lebih mudah / bersih / lebih cepat / lebih aman, dan secara fungsional tidak lebih buruk? Mungkin Anda menangkap keteraturan mendalam yang tidak tercakup oleh kode yang ada?
  • Apakah Anda melihat potensi untuk membuat abstraksi yang bermanfaat? Jika kode diduplikasi sebagian, ini sering berarti bahwa elemen fungsional atau abstrak yang lebih umum dapat diekstraksi darinya, yang kemudian dapat digunakan kembali dalam konteks lain.
  • Berpikirlah seperti lawan, tetaplah baik. Cobalah untuk "menangkap" penulis yang mengambil jalan pintas atau melewatkan kasus-kasus tertentu dengan melemparkan konfigurasi / input data bermasalah yang memecahkan kode mereka.
  • Pikirkan perpustakaan atau kode kerja yang sudah jadi. Jika seseorang mengimplementasikan kembali fungsionalitas yang ada - ini bukan hanya karena dia tidak tahu tentang keberadaan solusi yang sudah jadi. Terkadang kode atau fungsi sengaja diciptakan kembali - misalnya, untuk menghindari ketergantungan. Dalam hal ini, ini harus dikomentari dengan jelas dalam kode. Apakah fungsi ini sudah disediakan di perpustakaan yang ada?
  • Apakah perubahan itu sesuai dengan pola standar? Dalam basis kode yang ada, keteraturan sering dilacak terkait dengan konvensi penamaan, dekomposisi logika program, definisi tipe data, dll. Biasanya diinginkan bahwa semua perubahan diimplementasikan sesuai dengan pola yang ada.
  • Apakah dependensi yang terjadi selama kompilasi atau pada saat runtime (terutama antara sub-proyek) ditambahkan selama perubahan? Kami berusaha keras untuk koherensi yang lemah dari kode produk kami, yaitu, kami mencoba untuk memungkinkan minimum dependensi. Perubahan yang terkait dengan dependensi dan sistem pembangunan harus diperiksa dengan cermat.

Keterbacaan dan gaya


  • Pikirkan tentang bagaimana Anda membaca kode ini. Bisakah Anda memahami konsepnya cukup cepat? Apakah sudah ditata dengan wajar, apakah mudah untuk melacak nama-nama variabel dan metode? Bisakah saya melacak kode melalui beberapa file atau fungsi? Pernahkah Anda merasa terganggu oleh penamaan yang tidak konsisten di suatu tempat?
  • Apakah kode mematuhi pedoman pemrograman dan gaya yang terkenal? Apakah kode keluar dari seluruh proyek berdasarkan gaya, konvensi penamaan API, dll.? Seperti disebutkan di atas, kami mencoba untuk menyelesaikan perselisihan gaya menggunakan alat otomatis.
  • Apakah kode memiliki daftar tugas (TODO)? Daftar tugas hanya terakumulasi dalam kode dan akhirnya berubah menjadi pemberat. Tetapkan penulis untuk mengirimkan tiket ke Masalah GitHub atau JIRA dan melampirkan nomor tugas ke TODO. Seharusnya tidak ada kode komentar dalam perubahan yang diajukan.

Dukungan kegunaan


  • Baca tesnya. Jika tidak ada tes dalam kode, tetapi harus ada di sana, mintalah penulis untuk menulis beberapa. Fitur yang benar-benar belum diuji jarang terjadi, dan tidak diuji implementasi fitur - sayangnya, sering. Periksa tes sendiri: apakah mereka mencakup kasus yang menarik? Apakah nyaman untuk membacanya? Apakah cakupan tes keseluruhan menurun setelah tes ini? Pikirkan bagaimana kode ini bisa gagal. Standar desain uji dan kode inti seringkali berbeda, tetapi standar pengujian juga penting.
  • Apakah peninjauan fragmen ini berisiko melanggar kode uji, kode pembobolan, atau tes integrasi? Seringkali, tinjauan seperti itu tidak dilakukan sebelum komitmen / penggabungan, tetapi jika kasus-kasus seperti itu diabaikan, maka semua orang akan menderita. Perhatikan hal-hal berikut: menghapus utilitas atau mode uji, mengubah konfigurasi, mengubah tata letak / struktur artefak.
  • Apakah perubahan ini melanggar kompatibilitas ke belakang? Jika demikian, apakah mungkin untuk membuat perubahan ini ke rilis pada tahap ini, atau haruskah itu ditunda untuk rilis nanti? Pelanggaran dapat mencakup perubahan pada basis data atau skema, perubahan API publik, perubahan alur tugas di tingkat pengguna, dll.
  • Apakah kode ini memerlukan tes integrasi? Terkadang kode gagal diperiksa dengan benar menggunakan hanya unit test, terutama jika berinteraksi dengan sistem eksternal atau konfigurasi.
  • Tinggalkan umpan balik pada dokumentasi untuk kode ini, komentar dan pesan komit. Komentar luas menyumbat kode, dan berarti melakukan komit membingungkan mereka yang kemudian harus bekerja dengan kode. Ini tidak selalu terjadi, tetapi komentar berkualitas dan pesan komit dari waktu ke waktu pasti akan membantu Anda dengan baik. (Ingat ketika Anda melihat komentar yang indah atau hanya mengerikan atau komit pesan.)
  • Apakah dokumentasi eksternal diperbarui? Jika README, CHANGELOG, atau dokumentasi lainnya dipertahankan untuk proyek Anda, apakah diperbarui untuk mencerminkan perubahan yang dilakukan? Dokumentasi yang sudah usang mungkin lebih berbahaya daripada tidak sama sekali, dan akan lebih mahal untuk memperbaiki kesalahan di masa depan daripada membuat perubahan sekarang.

Jangan lupa untuk memuji kode singkat / mudah dibaca / efisien / indah. Sebaliknya, menolak atau menolak kode yang diusulkan untuk ditinjau tidak kasar. Jika perubahannya berlebihan atau tidak signifikan - tolak saja, jelaskan alasannya. Jika perubahan itu tampaknya tidak dapat Anda terima karena satu atau beberapa kesalahan fatal - tolak lagi, dengan alasan. Terkadang hasil terbaik dari tinjauan kode adalah mengatakan, "Mari kita lakukan dengan cara yang berbeda" atau "Jangan lakukan itu sama sekali."

Hargai rekan yang ditinjau. Meskipun posisi lawan terdengar baik, Anda tidak menyadari peluang ini, dan Anda tidak dapat memutuskan segalanya untuknya. Jika Anda tidak dapat mencapai pendapat peer-review tentang kode, mengatur konsultasi real-time dan mendengarkan pendapat orang ketiga.

Keamanan


Pastikan bahwa semua titik akhir API memiliki otorisasi dan otentikasi yang memadai sesuai dengan aturan yang diadopsi di seluruh basis kode. Cari kekurangan umum lainnya, misalnya: konfigurasi lemah, input pengguna jahat, entri log hilang, dll. Jika ragu, tunjukkan kode peer-review ke profesional keamanan.

Komentar: ringkas, sopan, insentif


Ulasan harus singkat, berkelanjutan dalam gaya netral. Mengkritik kode, bukan pembuatnya. Jika ada sesuatu yang tidak jelas - minta klarifikasi, dan jangan menganggap bahwa semuanya terletak pada ketidaktahuan penulis. Hindari kata ganti posesif, terutama dalam penilaian nilai: "kode saya berfungsi sebelum perubahan Anda ", "ada bug dalam metode Anda ", dll. Jangan memotong bahu: "itu tidak akan bekerja sama sekali ", "hasilnya selalu salah."

Cobalah untuk membedakan antara rekomendasi (misalnya, "Rekomendasi: ekstrak metode, ini akan membuat kode lebih jelas"), perubahan yang diperlukan (misalnya, "Tambah Timpa "), dan pendapat yang memerlukan klarifikasi atau diskusi (misalnya, "Apakah perilaku ini benar? ? Jika demikian, tambahkan komentar yang menjelaskan logikanya. ") Coba letakkan tautan atau petunjuk ke deskripsi masalah yang terperinci.

Ketika Anda menyelesaikan tinjauan kode, tunjukkan seberapa rinci reaksi terhadap komentar Anda yang Anda harapkan dari penulis, apakah Anda ingin mengulangi ulasan setelah perubahan dibuat, misalnya: "Anda dapat dengan aman mengunggah kode ke cabang utama setelah koreksi kecil ini" atau "Harap dicatat komentar saya dan beri tahu saya kapan saya bisa melihat kode lagi. "

Tinjau Tanggapan


Tinjauan kode, khususnya, dimaksudkan untuk meningkatkan permintaan untuk perubahan yang diajukan oleh penulis; karena itu, jangan menganggap permusuhan rekomendasi peninjau, menanggapinya dengan serius, bahkan jika Anda tidak setuju dengan rekomendasi tersebut. Tanggapi komentar apa pun, bahkan ke "ACK" yang biasa (disetujui) atau "selesai" (selesai). Jelaskan mengapa Anda membuat keputusan ini atau itu, mengapa Anda memiliki fungsi tertentu dalam kode Anda, dll. Jika Anda tidak dapat mencapai pendapat umum dengan pengulas, atur konsultasi secara real time dan dengarkan pendapat dari spesialis pihak ketiga.

Koreksi harus diperbaiki di cabang yang sama, tetapi dalam komit terpisah. Jika Anda tetap menggabungkan komit pada tahap review, akan lebih sulit bagi reviewer untuk melacak perubahan.

Kebijakan penggabungan kode berbeda untuk tim yang berbeda. Di beberapa perusahaan, penggabungan kode hanya dipercaya oleh pemilik proyek, di perusahaan lain, seorang peserta dapat melakukan ini setelah kodenya ditinjau dan disetujui.

Ulasan kode Tête-à-tête


Sebagai aturan, alat seperti-sinkron asinkron, seperti ditinjau, Gerrit, atau GitHub, bagus untuk ulasan kode. Jika kita berbicara tentang ulasan kompleks atau ulasan, yang pesertanya sangat berbeda dalam hal pengalaman atau profesionalisme, mungkin lebih efektif untuk melakukan tinjauan secara pribadi - baik duduk bersama di depan satu monitor atau proyektor, atau dari jarak jauh, melalui konferensi video atau alat berbagi layar.

Contohnya


Dalam contoh berikut, komentar resensi dalam cantuman dimasukkan menggunakan

 //R: ...


Penamaan yang tidak konsisten


 class MyClass { private int countTotalPageVisits; //R:    private int uniqueUsersCount; }

Tanda tangan metode yang tidak konsisten


 interface MyInterface { /**  {@link Optional#empty},  s   . */ public Optional<String> extractString(String s); /**  null,  {@code s}   . */ //R:    :    // Optional<> public String rewriteString(String s); }

Penggunaan perpustakaan


 //R:     MapJoiner   Guava String joinAndConcatenate(Map<String, String> map, String keyValueSeparator, String keySeparator);

Preferensi pribadi


 int dayCount; //R: :   numFoo   fooCount;   ,

Bug


 //R:    numIterations+1 –    ? //    –     numIterations? for (int i = 0; i <= numIterations; ++i) { ... }

Pertimbangan arsitektur


 otherService.call(); //R: ,      OtherService.    ?

Tentang penerjemah

Artikel ini diterjemahkan oleh Alconost.

Alconost melokalkan game , aplikasi , dan situs dalam 70 bahasa. Penerjemah asli bahasa, pengujian linguistik, platform cloud dengan API, pelokalan berkelanjutan, manajer proyek 24/7, segala format sumber daya string.

Kami juga membuat video iklan dan pelatihan - untuk situs yang menjual, gambar, iklan, pelatihan, permainan asah, penjelajah, trailer untuk Google Play dan App Store.

Lebih detail

Source: https://habr.com/ru/post/id413589/

More articles:


All Articles