Bagaimanakah Anda Menjalankan Code Review?

Prompt dan Konteks Berkenaan

Anda mempunyai 30 minit untuk menyemak pull request yang tidak dikenali. Terangkan cara anda membina semula niatnya, menyusun semakan, mengasingkan blocker daripada maklum balas bukan blocker, menulis komen yang boleh diambil tindakan oleh pengarang, dan memilih antara approve, comment, dan request changes.

Ini adalah soalan temu duga kejuruteraan perisian umum untuk peranan backend, frontend, mudah alih, infrastruktur dan pengurusan kejuruteraan. Ia mungkin ditanya sebagai soalan proses secara lisan atau sebagai semakan langsung terhadap diff yang dibekalkan. Kedua-dua bentuk menguji sama ada anda boleh mencari isu yang paling penting kepada pengguna dan sistem di bawah had masa, dan bukannya memaksimumkan bilangan kesalahan pemformatan yang anda laporkan.

Andaikan anda boleh melihat penerangan pull request, keperluan yang dipautkan, fail yang diubah dan keputusan ujian, tetapi anda tidak mengetahui pangkalan kod (codebase) dan tidak boleh terus-menerus menyoal pengarang. Jika penemu duga membekalkan syarat yang berbeza, tentukur semula risiko dan skop sebelum menyemak.

Perkara yang Dinilai oleh Penemu Duga

Isyarat pertama ialah sama ada anda membina semula apa yang sepatutnya dilakukan oleh perubahan tersebut. Tanpa keperluan, kontrak API atau sempadan kegagalan (failure boundary), komen akan merosot menjadi sekadar keutamaan peribadi. Jawapan yang mantap membaca konteks pull request dan kod sekeliling, mengenal pasti perubahan tingkah laku, dan hanya selepas itu menyemak baris demi baris.

Isyarat kedua ialah pengutamaan. Ketepatan (correctness), kerosakan data, keselamatan, kebenaran (authorization), keserentakan (concurrency) dan keserasian secara amnya patut diberi perhatian sebelum penamaan atau reka letak. Penemu duga ingin melihat sama ada anda boleh menyatakan akibatnya dan meluangkan masa pada laluan berisiko tertinggi.

Isyarat ketiga ialah bukti. "Ini mungkin pepijat" hanyalah satu petunjuk. Maklum balas berkualiti tinggi memberikan syarat pencetus, akibat yang boleh diperhatikan dan cara untuk mengesahkannya. Apabila konteks tiada, ia mengemukakan soalan yang tepat dan bukannya menyamar tekaan sebagai kesimpulan yang menyekat (blocking).

Akhir sekali, penemu duga menilai keputusan semakan dan komunikasi. Asingkan pembetulan yang diperlukan, cadangan pilihan, soalan penjelasan dan nit. Dalam ringkasan, nyatakan perkara yang anda bincangkan, perkara yang masih belum disahkan dan sebab anda meluluskan atau meminta perubahan. Bincangkan kod dan impaknya, bukan keupayaan pengarang.

Soalan untuk Dijelaskan Sebelum Menjawab

• Adakah ini soalan proses lisan atau semakan diff secara langsung? Untuk soalan lisan, bentangkan kaedah yang boleh diguna semula. Untuk semakan langsung, luangkan beberapa saat untuk menyatakan kaedah dan kemudian gunakannya pada baris sebenar dan bukannya membaca senarai semak.
• Apakah konteks yang tersedia? Keperluan, kontrak API dan kod sekeliling membolehkan anda mengesahkan tingkah laku. Dengan fungsi terpencil, nyatakan andaian dan tukar kontrak yang tidak diketahui menjadi soalan.
• Adakah perubahan itu menyentuh domain berisiko tinggi? Pembayaran, kebenaran (authorization), privasi, migrasi dan API awam meningkatkan standard bukti dan mungkin memerlukan pakar domain. Alat dalaman berisiko rendah boleh mengutamakan penambahbaikan bertahap yang lebih pantas.
• Apakah hasil (deliverables) yang diharapkan oleh penemu duga? Komen sebaris (inline comments), ringkasan, keputusan kelulusan dan pengesyoran ujian memerlukan peruntukan masa yang berbeza. Sahkan output sebelum menghabiskan masa 30 minit tersebut.
• Adakah ini kerja biasa atau pembetulan kecemasan? Kecemasan boleh mewajarkan patch yang lebih sempit dan kerja susulan. Ia tidak mewajarkan pengabaian risiko keselamatan atau kerosakan data yang diketahui.
• Adakah anda layak untuk setiap domain yang terjejas? Jika perubahan melibatkan kriptografi, privasi atau migrasi pangkalan data di luar kepakaran anda, semak perkara yang anda boleh dan minta penyemak yang berkelayakan dan bukannya meluluskan berdasarkan keyakinan semata-mata.

Rangka Kerja Jawapan 30 Saat

"Mula-mula saya menetapkan matlamat pull request, perubahan tingkah laku dan impak kegagalan, kemudian menyemak dalam dua pusingan. Pusingan pertama memetakan sempadan perubahan, aliran data dan laluan berisiko tinggi, mengutamakan ketepatan, keselamatan, data, keserentakan dan keserasian. Pusingan kedua menyemak kes pinggir (edge cases), pengendalian ralat, ujian, kebolehcerapan (observability), prestasi dan kebolehselenggaraan. Setiap komen menyatakan keterukan, pencetus, akibat dan hasil yang dijangkakan. Blocker yang boleh dihasilkan semula bermakna request changes; cadangan dan nit boleh disertakan bersama kelulusan (approve). Saya akhiri dengan skop yang disemak, risiko yang belum disahkan dan sebab keputusan saya."

Jawapan Mendalam Langkah demi Langkah

Wujudkan garis dasar (baseline) terlebih dahulu. Baca tajuk, penerangan, keperluan yang dipautkan, perubahan API atau model data, dan ujian sedia ada. Nyatakan semula kontrak dalam satu ayat: "Perubahan ini memberi pengguna ini tingkah laku baharu di bawah syarat-syarat ini sambil mengekalkan jaminan sedia ada ini." Jika anda tidak boleh menulis ayat itu, dapatkan konteks sebelum memberikan komen peringkat baris kerana anda belum mempunyai standard ketepatan.

Petakan sempadan perubahan seterusnya. Ikuti input, perubahan keadaan (state), kesan sampingan luaran dan laluan kembali (return paths) dan bukannya hanya melihat baris yang diserlahkan. Pemanggil manakah yang menerima parameter baharu? Adakah penulisan pangkalan data dan penghantaran mesej berada dalam sempadan kegagalan (failure boundary) yang sama? Adakah respons awam, format peristiwa (event) atau lalai konfigurasi berubah? Output pusingan ini ialah model tempat input masuk, sempadan amanah (trust boundaries) yang dilintasinya, perubahan state dan cara ia boleh gagal.

Untuk contoh kotak masa 30 minit, gunakan 3 minit untuk niat, 7 minit untuk sempadan dan laluan berisiko tinggi, 12 minit untuk pemeriksaan terperinci, 5 minit untuk ujian dan perlindungan operasi, dan 3 minit untuk komen dan keputusan. Ini adalah peruntukan latihan untuk diselaraskan mengikut saiz diff dan risiko. Tujuannya adalah untuk mengelakkan daripada menghabiskan 20 minit pertama untuk penamaan.

Gunakan urutan risiko ini dalam pusingan pertama:

1. Tingkah laku dan ketepatan: Adakah laluan utama memenuhi kontrak? Apakah yang berlaku dengan input kosong, permintaan pendua, kegagalan separa dan percubaan semula (retries)?
2. Keselamatan dan data: Adakah kebenaran (authorization) berlaku sebelum melintasi sempadan amanah? Adakah data sensitif terdedah? Bolehkah kegagalan menyebabkan kehilangan, pertindihan atau keadaan yang tidak boleh diubah?
3. Keserentakan dan keserasian: Bolehkah permintaan serentak merosakkan invarian? Adakah klien lama, data lama dan versi bercampur semasa rolling deployment masih berfungsi?
4. Sempadan seni bina: Adakah tanggungjawab berada dalam komponen yang betul, atau adakah perubahan itu memintas kekangan sedia ada dan menduplikasi keadaan (state)?

Pusingan kedua memeriksa perincian pelaksanaan: aliran kawalan dan penyebaran ralat, pembersihan sumber, skala pertanyaan atau gelung, log dan metrik, sama ada ujian benar-benar akan gagal apabila kod itu salah, dan sama ada nama dan komen membantu pembaca masa hadapan. Letakkan pemformatan dan gaya yang boleh diperbaiki secara automatik di tempat terakhir supaya nit yang boleh dikesan oleh alat tidak menggantikan pertimbangan manusia.

Bagi setiap penemuan, sahkan bahawa perubahan itu memperkenalkan atau mendedahkannya. Jika kod baharu membaca items[0] apabila items kosong adalah sah, itu adalah regresi yang konkrit. Jika fail yang sama mengandungi kerumitan sedia ada yang tidak berkaitan, sebutkannya sebagai hutang teknikal atau buat kerja susulan melainkan interaksinya dengan perubahan ini mewujudkan risiko keselamatan atau ketepatan. Semakan tidak boleh berkembang tanpa sempadan.

Gunakan empat niat komen:

• Blocker: Bukti menunjukkan pelanggaran kontrak, hasil yang salah, isu keselamatan, kerosakan data atau risiko keserasian yang tidak boleh diterima. Ia mesti diselesaikan sebelum penggabungan (merge).
• Question: Konteks yang boleh mengubah kesimpulan tiada. Jawapannya boleh menutup kebimbangan atau menaikkannya kepada Blocker.
• Suggestion: Penambahbaikan reka bentuk, kebolehselenggaraan atau operasi yang berbaloi, manakala pelaksanaan semasa masih memenuhi standard penggabungan.
• Nit: Perincian kebolehbacaan atau ketekalan bukan blocker yang idealnya perlu dikendalikan oleh pemformatan atau analisis statik.

Komen yang boleh diambil tindakan mengandungi "label + syarat + akibat + hasil yang dijangkakan," diikuti dengan satu kemungkinan arah penyelesaian apabila berguna. Sebagai contoh:

Blocker: Apabila permintaan membenarkan items=[], membaca items[0].id di sini membuang exception dan endpoint kelompok mengembalikan 500. Sila kendalikan array kosong sebelum gelung dan tambah ujian regresi; sama ada untuk mengembalikan hasil kosong atau 400 bergantung pada kontrak API.

Jika anda tidak tahu sama ada input kosong dibenarkan, jadikannya soalan: "Adakah kontrak API membenarkan array kosong? Laluan semasa mengembalikan 500; jika ia dibenarkan, ia memerlukan pengendalian eksplisit dan ujian." Ini melaporkan bukti tanpa mereka-reka keperluan.

Selesaikan dengan keputusan semakan. Pilih request changes apabila Blocker belum selesai. Hantar komen apabila konteks penting tiada dan bukannya menyembunyikan ketidakpastian di sebalik kelulusan. Luluskan (approve) apabila hanya cadangan bukan blocker yang tinggal, dan nyatakan bahawa ia bukan syarat penggabungan. Ringkasan harus menamakan skop yang disemak, penemuan utama, bukti masa jalan (runtime) atau ujian, domain yang tidak diliputi dan status akhir.

CI hijau tidak membuktikan bahawa semakan telah selesai. Ujian mungkin meninggalkan cabang kritikal, dan alat statik tidak mengetahui kontrak produk. Sebaliknya, semakan manusia tidak seharusnya menggantikan ujian yang boleh dilaksanakan. Sambungkan kedua-duanya: kenal pasti syarat kegagalan dalam komen dan minta semakan yang gagal sebelum pembetulan dan lulus selepas itu.

Contoh Jawapan Berkualiti Tinggi

"Saya tidak akan bermula dengan memburu kesalahan peringkat baris. Saya akan membaca penerangan pull request, keperluan yang dipautkan dan perubahan antara muka, kemudian menyatakan semula matlamat dan tingkah laku lama yang mesti kekal benar. Jika konteks tidak tersedia, saya akan menyenaraikan andaian dan bukannya mengemukakan tekaan sebagai blocker.

Dalam masa 30 minit, saya akan menggunakan dua pusingan. Pusingan pertama mengikut titik masuk, perubahan keadaan, kesan sampingan luaran dan laluan kembali, mengutamakan ketepatan, keselamatan, kerosakan data, keserentakan dan keserasian. Pusingan kedua merangkumi kes pinggir, pengendalian ralat, prestasi, log, ujian dan kebolehselenggaraan. Gaya berada di tempat terakhir, dan hanya apabila perkakasan tidak meliputi isu yang benar-benar menjejaskan pemahaman.

Setiap penemuan mesti menjawab tiga soalan: perkara yang mencetuskannya, apakah akibatnya dan cara mengesahkannya. Saya melabelkan pembetulan yang diperlukan sebagai Blocker, konteks yang hilang sebagai Question, penambahbaikan bukan blocker sebagai Suggestion dan kemasan kecil sebagai Nit. Sebagai contoh, jika laluan array kosong membaca elemen pertama walaupun API menerima input kosong, saya akan menerangkan bahawa ia mengembalikan 500 dan meminta pengendalian eksplisit ditambah ujian regresi, dan bukannya menulis hanya 'kemungkinan isu null.'

Sebelum menghantar, saya menyemak bahawa setiap komen berkaitan dengan diff ini, bahawa saya tidak mengangkat keutamaan peribadi menjadi peraturan, dan tiada penyemak domain yang diperlukan tertinggal. Masalah keselamatan, data atau ketepatan yang boleh dihasilkan semula membawa kepada request changes; cadangan sahaja boleh mengiringi kelulusan. Ringkasan saya menyenaraikan fail yang diliputi, bukti ujian, kawasan yang belum disahkan dan rasional keputusan supaya pengarang mengetahui tindakan seterusnya dan penyemak kemudian mengetahui perkara yang sebenarnya saya semak."

Kesilapan Biasa

• Membuka diff dan memberi komen baris demi baris → Tanpa niat atau kontrak, kompromi (tradeoff) yang sah kelihatan seperti kesalahan → Nyatakan semula matlamat, perubahan tingkah laku dan sempadan kegagalan terlebih dahulu.
• Memberi komen mengikut urutan penemuan → Perincian penamaan boleh mengaburkan risiko kerosakan data atau kebenaran → Jalankan pusingan risiko sebelum kemasan pelaksanaan.
• Hanya menulis "ini mungkin pepijat" → Pengarang kekurangan pencetus dan tidak dapat mengesahkan pembetulan → Nyatakan syarat, akibat, bukti dan hasil yang dijangkakan.
• Menjadikan setiap komen wajib → Pengarang tidak dapat membezakan standard penggabungan daripada keutamaan peribadi → Labelkan Blocker, Question, Suggestion dan Nit secara eksplisit.
• Membaca senarai semak yang panjang untuk kelihatan teliti → Senarai yang tidak digunakan pada aliran data menunjukkan sedikit pertimbangan → Jejaki satu laluan kritikal dan terangkan liputan yang selebihnya.
• Menuntut agar semua masalah lama diperbaiki → Pull request berkembang tanpa risiko atau pengesahan yang terikat sempadan → Asingkan regresi daripada hutang sedia ada kecuali jika ia bergabung menjadi isu keselamatan atau ketepatan.
• Menganggap CI hijau sebagai bukti kelulusan → Ujian mungkin mengekodkan kontrak yang salah atau meninggalkan satu cabang → Semak sama ada ujian gagal untuk contoh lawan (counterexample) utama dan minta ujian regresi yang hilang.
• Mengulas mengenai pengarang dan bukannya kod → Ia mewujudkan sikap defensif dan tidak membekalkan rasional teknikal → Huraikan kod, syarat dan impak sambil menganggap niat yang baik.
• Meluluskan di luar kepakaran anda → Kelulusan itu mewujudkan jaminan palsu → Nyatakan liputan anda dan minta penyemak domain.

Soalan Susulan dan Maklum Balas

Susulan 1: Bagaimana jika pengarang mempertikaikan Blocker anda?

Kembali kepada kontrak dan akibat yang boleh disahkan. Semak sama ada anda tidak bersetuju tentang input, risiko atau syarat pelepasan, dan gunakan pengeluaran semula minimum (minimum reproduction) apabila boleh. Jika bukti tidak menghasilkan konsensus, minta pemilik kod atau pemilik domain untuk membuat keputusan dan rekodkan sebarang kesimpulan lisan dalam pull request. Jangan biarkan perselisihan berterusan tanpa had.

Susulan 2: Bagaimana jika pull request terlalu besar untuk diselesaikan dalam masa 30 minit?

Jangan membayangkan liputan penuh. Pilih titik masuk, perubahan data dan kontrak awam mengikut risiko; nyatakan fail yang anda semak baris demi baris, imbas sepintas lalu atau tidak diliputi; kemudian minta pemisahan atau penyemak domain tambahan. Status kelulusan mesti sepadan dengan liputan yang sebenarnya dilakukan.

Susulan 3: Bagaimana jika anda mendapati isu sedia ada yang teruk di luar diff?

Mula-mula tentukan sama ada perubahan ini mencetuskan atau menguatkannya. Sekat apabila interaksi itu mewujudkan risiko keselamatan, data atau ketepatan untuk pelepasan semasa. Jika ia bebas, rekodkan bukti, buat kerja susulan berkeutamaan tinggi dan maklumkan kepada pemilik dan bukannya memaksa refaktor tanpa sempadan ke dalam pull request ini.

Susulan 4: Bolehkah anda meluluskan pembetulan kecemasan tanpa ujian lengkap?

Tetapkan kos segera jika tidak diperbaiki, sama ada patch itu sempit, laluan rollback atau suis ciri (feature switch), dan pengesahan disasarkan terkecil yang tersedia. Proses kecemasan yang jelas mungkin menerima liputan susulan, tetapi risiko keselamatan, kerosakan data atau risiko tidak boleh balik yang diketahui masih memerlukan tahap kelulusan yang lebih tinggi. Had masa tidak meluluskannya secara automatik.

Susulan 5: Bagaimanakah anda menyemak domain yang anda tidak tahu?

Teruskan menyemak aliran kawalan umum, pengendalian ralat, ujian dan perubahan antara muka sambil menandakan perkara yang anda tidak layak untuk nilai. Minta pemilik yang sesuai untuk kriptografi, privasi, migrasi atau keserentakan yang kompleks. Semakan separa hanya bernilai apabila ia tidak diwakili sebagai kelulusan penuh.

Susulan 6: Adakah anda masih perlu membaca setiap baris apabila liputan ujian adalah meluas?

Ya, dengan fokus yang berbeza. Ujian menyediakan bukti yang boleh dilaksanakan untuk kes yang dikodkan; semakan masih menyoal sama ada keperluan itu betul, risiko terlepas pandang, reka bentuk menambah kerumitan yang tidak perlu, dan pengelogan atau keserasian adalah sesuai. Tukar contoh lawan penting yang ditemui dalam semakan menjadi ujian supaya ketepatan masa hadapan tidak bergantung pada ingatan penyemak.