Alur kerja tinjauan kode di mana penulis permintaan tarik harus bergabung

16

Beberapa tim di perusahaan saya mempraktikkan alur kerja tinjauan kode yang belum pernah saya lihat sebelumnya. Saya mencoba memahami pemikiran di baliknya, dengan gagasan bahwa ada nilai dalam membuat seluruh perusahaan konsisten. (Saya berkontribusi pada banyak basis kode dan telah tersandung oleh perbedaan di masa lalu.)

  1. Penulis kode mengajukan permintaan tarik
  2. Reviewer memeriksa kodenya
    • Jika peninjau menyetujui, mereka memberikan komentar di sepanjang baris "Terlihat bagus, jangan ragu untuk bergabung"
    • Jika pengulas memiliki masalah, mereka memberikan komentar seperti "Harap perbaiki masalah kecil X dan Y, lalu gabungkan" (Untuk perubahan besar, kembali ke langkah 2)
  4. Pembuat kode melakukan perubahan jika perlu, dan kemudian menggabungkan permintaan tariknya sendiri

Saya memiliki masalah berikut:

  • Dalam hal persetujuan pada langkah 3, alur kerja ini membuat bolak-balik yang tampaknya tidak perlu ke penulis permintaan tarik. Peninjau, yang sudah melihat kode, bisa langsung menggabungkannya.

  • Dalam kasus perubahan yang diminta pada langkah 3, agensi untuk menggabungkan permintaan tarik sekarang sepenuhnya berada di tangan penulis PR. Tidak seorang pun selain penulis akan melihat perubahan sebelum penggabungan.

Apa beberapa keuntungan atau kerugian lain dari alur kerja ini? Apakah alur kerja ini umum di tim teknik lain?

code-reviews pull-requests aednichols
sumber
5
Sudahkah Anda bertanya kepada orang-orang di organisasi Anda mengapa mereka memilih alur kerja khusus ini? Mereka mungkin berada dalam posisi yang lebih baik untuk menjelaskan manfaat yang relevan daripada kita. Kami hanya akan berspekulasi.
Robert Harvey
1
Apa yang terjadi di organisasi Anda ketika pengulas menulis "Tolong perbaiki masalah besar X"?
Doc Brown
8
Dalam pengalaman saya, yang terbaik bagi penulis asli untuk menjadi orang yang melakukan penggabungan jika ada konflik penggabungan yang harus diselesaikan. Penulis asli biasanya adalah orang yang paling siap untuk mencari tahu bagaimana menyelesaikan konflik gabungan.
17 dari 26
Saya ingin tahu logika di sini. Anda harus bertanya kepada kolega Anda dan menuliskannya sebagai jawaban sendiri - mungkin ada proses pemikiran atau pemikiran yang sangat bagus di sini. Saya hanya tidak dapat menemukan satu dengan cepat.
Thomas Owens

Jawaban:

21

Dalam kasus pertama, biasanya merupakan rasa hormat. Di sebagian besar organisasi, gabungan memulai serangkaian tes otomatis yang harus segera ditangani jika gagal. Terutama jika ada penundaan yang signifikan antara saat permintaan tarik diajukan dan ketika ditinjau, sopan untuk membiarkannya digabungkan pada jadwal penulis, sehingga mereka punya waktu untuk berurusan dengan kejatuhan yang tidak terduga. Cara termudah untuk melakukannya adalah membiarkan mereka menggabungkannya sendiri.

Selain itu, kadang-kadang penulis menyadari alasan di kemudian hari bahwa permintaan tarik tidak boleh digabungkan. Mungkin PR pengembang lain adalah prioritas yang lebih tinggi dan akan menyebabkan konflik. Mungkin dia memikirkan kasus penggunaan yang tidak tertutup. Mungkin komentar ulasan memicu brainstorming yang perlu diselidiki lebih lanjut sebelum masalah terpenuhi. Penulis tahu paling banyak tentang kode, dan masuk akal untuk memberinya kata terakhir tentang kapan kode tersebut digabungkan.

Pada poin kedua, itu hanya masalah kepercayaan. Jika Anda tidak dapat mempercayai orang untuk memperbaiki masalah kecil tanpa diperiksa ulang, mereka seharusnya tidak bekerja untuk Anda. Jika masalah ini cukup besar sehingga perlu ditinjau lagi setelah perbaikan, percayai pengulas untuk meminta satu.

Yang sedang berkata, saya kadang-kadang menggabungkan permintaan tarik penulis lain, tetapi biasanya perubahan yang sangat sederhana, atau dari sumber eksternal, di mana saya secara pribadi bertanggung jawab untuk penggembalaan melalui setiap kegagalan otomasi pengujian.

Karl Bielefeldt
sumber
2
Kedengarannya ada lebih banyak variasi di luar sana daripada yang saya bayangkan. Pengalaman saya dengan pengujian otomatis adalah bahwa pengujian dijalankan terhadap cabang sebelum digabungkan, bukan setelahnya, sehingga menjadi prasyarat untuk penggabungan. Saya juga telah melihat perbaikan "minor" yang tidak ditinjau, termasuk saya sendiri, yang menyebabkan bug.
aednichols
2
Tes sering dijalankan sebagai kondisi akhir dan juga kondisi awal. Sangat mungkin untuk perubahan dari beberapa cabang menjadi konflik dengan cara yang tidak jelas yang tidak akan muncul sebagai konflik kode dan menyebabkan pengujian mulai gagal. Di tempat kerja saya, kami membutuhkan cabang untuk memperbarui dengan cabang dasar serta semua tes yang lewat sebelum itu merupakan kandidat untuk penggabungan dan penggabungan terjadi secara otomatis jika kedua kondisi tersebut terpenuhi. Kami tidak selalu memiliki kondisi pertama itu - sebelum itu kami benar-benar memiliki masalah yang diperkenalkan ke master jarang meskipun setiap cabang disahkan secara individual
Matthew Scharley
3

Memiliki penulis awal menggabungkan permintaan tarik mereka sendiri adalah alur kerja pilihan saya di tim kecil. Selain keuntungan teknis yang telah disebutkan (dalam hal menyelesaikan konflik gabungan, misalnya), saya pikir itu menambah nilai pada tingkat budaya: Ini membangun rasa kepemilikan.

Saya menetapkan penulis awal untuk kasus (jarang) ketika pengembang lain akan menambahkan komit ke permintaan tarik terbuka (menarik cabang fitur work-in-progress dan mendorong kembali ke sana). Ini tidak sering terjadi, dan akan dihasilkan dari percakapan langsung atau lebih dari Slack: Komitmen tambahan ini (oleh orang lain) tidak boleh mendarat di sana dengan kejutan! Dalam konteks ini, menurut penulis awal , yang saya maksud adalah yang mengajukan permintaan tarik.

mkcor
sumber
2

Di organisasi saya, kami cukup baru dalam menarik permintaan dan pertanyaan Anda adalah satu yang telah saya renungkan sendiri.

Satu pengamatan yang ingin saya tambahkan: Dalam beberapa alat (kami menggunakan TFS) mungkin ada item pekerjaan yang terkait dengan permintaan tarik.

Jika itu masalahnya, menjadi sedikit repot untuk melacak kapan peninjau melakukan penggabungan. Dalam skenario itu pengembang kemudian harus mengunjungi kembali PR, buka bug atau ubah permintaan dan tandai sebagai 'diselesaikan'. Jika kami menandainya 'diselesaikan' terlalu dini, penguji yakin bahwa perbaikan sudah menjadi bagian dari pembangunan saat ini.

TFS 2017 meningkat pada penerapan permintaan tarik mereka. Sekarang pengembang dapat meminta Permintaan Tarik untuk bergabung secara otomatis jika semua persyaratan terpenuhi (tidak ada konflik gabungan, persetujuan dari pengulas, dan tidak ada bangunan yang rusak). YMMV.

9Rune5
sumber
1

Dengan cara ini, penulis memiliki kesempatan untuk berubah pikiran tentang cabangnya, mungkin dia menemukan sesuatu yang harus dilakukan dengan cara yang berbeda, dan mengembalikan tagihan tambahan untuk ditinjau.

gnasher729
sumber