Proses Review Pull Request

Sebagai aplikasi open source di Github, siapapun dapat mengirim perubahan pada script OpenSID. Dan sebagai aplikasi yang dikembangkan oleh komunitas pegiat dan pengguna, kita secara aktif mengajak semua yang berminat untuk kontribusi mengembangkan OpenSID.

Perubahan tersebut dikirim ke OpenSID melalui pull request (PR). Petunjuk ini menjelaskan bagaimana PR direview sebelum digabungkan ke script OpenSID.

Mengapa PR perlu direview

PR dikirim oleh pegiat2 dengan latar belakang dan penguasaan teknis yang berbeda-beda. Kebanyakan umumnya belum terbiasa membuat kontribusi pada proyek open source seperti OpenSID. Para programmer yang mengirim PR juga mempunyai kebiasaan koding yang berbeda-beda. Perlu diantisipasi, PR yang dikirim oleh para kontributor kualitasnya akan beragam, dan perlu diperiksa untuk menentukan apakah layak untuk digabung ke rilis OpenSID atau tidak.

Tim Rilis OpenSID bertanggung jawab menjaga supaya script OpenSID mengikuti standar yang telah kita terapkan -- dengan tujuan supaya menjadi semakin mudah dimengerti dan mudah diubah. PR perlu direview supaya:

• hanya perubahan yang memang dibutuhkan digabungkan ke rilis
• perubahan telah diuji-coba dan sejauh mungkin tidak mengandung error
• rancangan script terjaga dibuat mudah dimengerti dan mudah diubah, yaitu menganut cara koding yang baik
• penulisan script mengikuti aturan penulisan script yang telah ditentukan untuk OpenSID

Langkah review PR

1. Setiap PR perlu terkait suatu issue
2. Setiap PR perlu dibatasi satu issue saja
3. Yakinkan issue tersebut masih berlaku dan memang perlu dikerjakan
4. Perubahan file PR harus dibatasi pada file yg diubah saja
5. Perubahan isi file harus dibatasi pada script yg diubah saja
6. Kalau PR memerlukan perubahan struktur database, perlu ada migrasi
7. PR perlu berjalan sukses di lokal
8. PR perlu berjalan sukses untuk berbagai skenario penggunaan terkait issue
9. Penulisan script perlu sesuai dengan aturan penulisan script OpenSID
10. Pengaturan file dan method script perlu sesuai dengan praktik baik koding
11. Praktik baik keamanan data telah diterapkan
12. Selesaikan review untuk meminta PR diperbaiki
13. Sekalian lakukan perbaikan lain kalau sempat
14. Mutakhirkan catatan rilis
15. Commit perubahan yang dibuat oleh pe-review
16. Gabungkan PR ke branch master
17. Muat ke situs https://demosid.opendesa.id/

Setiap PR perlu terkait suatu issue

Setiap perubahan OpenSID yang berasal dari publik perlu terkait suatu kebutuhan yang jelas. Kebutuhan tersebut terrekam di issue di https://github.com/OpenSID/OpenSID/issues. Kalau ada PR yang tidak terkait issue, pengirim diminta untuk membuat issue dulu sebelum review dapat dilanjutkan.

Setiap PR perlu dibatasi satu issue saja

PR yang menerapkan beberapa issue sekaligus sulit direview. Pada prakteknya, ada kemungkinan solusi untuk suatu issue lolos review, sedangkan solusi untuk issue lainnya perlu perbaikan. Keadaannya ini akan membuat proses review dan penggabungan script rumit. Karena itu, kalau ada PR yang berisi solusi untuk beberapa issue sekaligus, pengirim diminta untuk memisahkan menjadi PR tersendiri untuk masing-masing issue. Yaitu, review untuk PR gabungan tidak bisa dilanjutkan.

Catatan: Pada kasus tertentu, untuk Setiap PR perlu terkait suatu issue dan Setiap PR perlu dibatasi satu issue saja tidak dilakukan dengan kondisi terdapat issue yang saling berkaitan dan tidak dapat dikerjakan secara terpisah. Misalnya pada OpenSID kasus di atas bisa terjadi jika baru mulai mengerjakan/merobak modul yang berkaitan dengan Autentikasi dimana memerlukan modul-modul seperti user, role dan group..

Yakinkan issue tersebut masih berlaku dan memang perlu dikerjakan

Yang melakukan review perlu memeriksa issue terkait untuk meyakinkan issue tersebut masih berlaku dan memang perlu dikerjakan. Ada kalanya pengirim mengerjakan issue yang sudah dikerjakan sebelumnya dalam bentuk lain, atau memang issue yang bersangkutan belum sempat direview dan disetujui karena berbagai hal. Dalam hal ini, yang perlu dilakukan adalah:

• beritahu pengirim bahwa issue terkait tidak akan dikerjakan dan PRnya tidak akan digabung dan akan ditutup
• tambahkan comment di issue terkait menjelaskan mengapa issue tersebut tidak disetujui dan kemudian tutup issue tersebut.

Perubahan file PR harus dibatasi pada file yg diubah saja

Pengirim PR terkadang belum perpengalaman mengirim PR. Situasi yang terkadang ditemukan:

• tidak ada file yang diubah, di mana pengirim melampirkan file kiriman di dalam comment
• banyak file yang diubah, termasuk file yang tidak ada kaitannya dengan issue yang sedang dikerjakan

[Gambar melihat file di PR]

Apabila keadaan di atas ditemukan, pe-review perlu melakukan:

• meminta pengirim untuk mempelajari lagi cara mengirim PR
• menganjurkan pengirim melihat contoh PR lain yang sudah benar
• mengusulkan agar pengirim konsultasi dengan pegiat lain yang dapat membantu
• meminta pengirim untuk mengirim PR susulan setelah diperbaiki

Perubahan isi file harus dibatasi pada script yg diubah saja

Ada kalanya semua script berubah dalam file yang dikirim dalam PR. Hal ini bisa terjadi karena pengirim menggunakan editor yang mengubah semua script secara otomatis, atau pengirim mengubah format indentasi atau lainnya. Perubahan seperti ini akan sulit direview, karena kita tidak bisa melihat apa saja yang sebenarnya telah diubah oleh pengirim.

[Gambar contoh semua script di file terubah]

Kalau keadaan ini ditemukan, minta pengirim untuk mengirim ulang PR-nya supaya review dapat dilanjutkan.

Kalau PR memerlukan perubahan struktur database, perlu ada migrasi

Sering pengirim belum mengetahui cara untuk melakukan perubahan database melalui proses migrasi yang digunakan OpenSID, sehingga pengirim tidak mengikutsertakan perubahan database atau mengirim contoh data awal yang telah mereka ubah. Untuk kasus seperti ini, beritahu pengirim petunjuk menambahkan migrasi (Menambah-Migrasi-Database) dan minta mereka menambahkan migrasi DB ke PR susulan supaya review dapat dilanjutkan.

Periksa migrasi dengan teliti. Pastikan migrasi:

1. Dapat dijalankan berulang-kali
2. Mengantisipasi dampak pada data desa yang sudah ada. Misalnya, jika menambah indeks unik pada suatu kolom, apakah yang terjadi kalau isi kolom tersebut tidak unik
3. Kalau membuat kolom baru, kolom tersebut telah diberikan jenis data, panjang kolom dan nilai default yang benar. Periksa juga pernyataan boleh unik dan boleh null sudah benar

PR perlu berjalan sukses di lokal

Pasang PR di komputer lokal untuk diuji coba. Yakinkan perubahan di PR ini betul-betul merupakan solusi untuk issue terkait. Kalau ditemukan masalah, beritahu pengirim PR dan minta mereka perbaiki.

Untuk memasang PR di komputer lokal ikuti langkah-langkah berikut:

1. Copas perintah untuk pull PR ke branch baru [gambar copy di Github] [gambar paste di terminal]
2. Jalankan Migrasi DB

PR perlu berjalan sukses untuk berbagai skenario penggunaan terkait issue

Antisipasi berbagai skenario terkait PR, termasuk memasukkan data salah, melakukan langkah tidak terduga, menjalankan menu/fitur yang mungkin terdampak oleh perubahan di PR. Yakinkan tidak ada dampak tidak terduga dari PR.

Kalau ditemukan masalah, beritahu pengirim PR dan minta mereka perbaiki.

Penulisan script perlu sesuai dengan aturan penulisan script OpenSID

Periksa perubahan pada setiap file di PR. Yakinkan semua script yang ditambah atau diubah mengikuti aturan penulisan script di https://github.com/OpenSID/OpenSID/wiki/Aturan-Penulisan-Script.

Kalau ditemukan masalah, beritahu pengirim PR dan minta mereka perbaiki. Cara memberitahu masalah terkait baris tertentu di script dijelaskan berikut ini:

1. Di Github, periksa file di PR [gambar periksa file PR]
2. Scroll ke baris terkait
3. Klik tanda '+'
4. Isi comment

Pengaturan file dan method script perlu sesuai dengan praktik baik koding

Selain penulisan script, pe-review perlu juga memeriksa struktur script, method dan file yang ditambah atau diubah oleh PR. Yakinkan semua script mengikuti praktik baik koding, termasuk:

• penamaan variabel yang baik
• tidak ada duplikasi script
• method diletakkan di file yang sesuai
• tidak ada method yang terlalu panjang
• script yang rumit menggunakan struktur yang mudah dimengerti
• script yang rumit diberi komen menjelaskan cara bekerja script
• dsbnya

Kalau ditemukan masalah, beritahu pengirim PR dan minta mereka perbaiki.

Praktik baik keamanan data telah diterapkan

OpenSID digunakan oleh desa untuk menyimpan data yg perlu dijamin keamanannya. Untuk setiap perubahan, periksa apakah celah keamanan telah ditutup, termasuk hal2 berikut:

1. Tidak menggunakan mass assignment pada waktu menyimpan data form ke database. Yaitu jangan simpan post secara utuh, tapi hanya simpan data yg memang perlu disimpan (ambil satu-per-satu)
2. Di form lakukan validasi di browser (jquery validate) dan juga di server. Sterilkan data sebelum disimpan di database sesuai dengan spesifikasi masing2 kolom isian. Misalnya, pastikan javascript atau tag HTML tidak tersimpan bagi kolom isian yg seharusnya tidak berisi unsur itu.
3. Jika menggunggah file, pastikan hanya jenis file yg diperbolehkan yg diunggah. Pastikan file berisi script PHP tidak dapat diunggah.
4. Gunakan query builder CI, dan bukan native SQL, supaya otomatis menggunakan fitur anti-sql-injection dari CI.

Selengkapnya : https://github.com/OpenSID/OpenSID/wiki/Keamanan

Kalau ditemukan masalah, beritahu pengirim PR dan minta mereka perbaiki.

Selesaikan review untuk meminta PR diperbaiki

Apabila PR perlu diperbaiki dan belum siap untuk digabung, beritahu pengirim dan selesaikan review mengikut cara sebagai berikut:

1. Beri komentar seperlunya [gambar contoh mengisi komenar di github]
2. Selesaikan review dengan meminta perubahan PR [gambar menyelesaikan review]

Sekalian lakukan perbaikan lain kalau sempat

Pada saat memeriksa file yang diubah, ada kalanya ditemukan masalah lain di script yang tidak secara langsung terkait PR atau issue yang sedang dikerjakan. Kalau sempat, dianjurkan pe-review sekalian saja melakukan perbaikan supaya script OpenSID bisa terus membaik dan bertambah mapan.

Mutakhirkan catatan rilis

Sesudah PR lolos review, mutakhirkan catatan rilis. Tambahkan keterangan singkat mengenai issue yang telah dikerjakan. Tambahkan juga nama pegiat yang mengirim PR.

Commit perubahan yang dibuat oleh pe-review

Yang melakukan review umumnya perlu juga melakukan perubahan, termasuk untuk:

• memperbaiki hal-hal yang lebih cepat langsung diperbaiki daripada meminta pengirim PR mengirim PR ulang
• mengubah script yang sebaiknya diubah, tapi bukan terkait langsung dengan issue atau PR
• memutakhirkan catatan rilis

Perubahan yang dilakukan oleh pe-review perlu dicommit ke branch yang sedang direview.

Gabungkan PR ke branch master

Sesudah semua perubahan telah dicommit, branch yang sedang direview perlu digabung ke master. Setelah digabung, branch master perlu dipush ke Github. Setelah dipush ke Github, hasil PR tersebut langsung tersedia untuk diunduh oleh siapa saja.

Push perubahan tema natra

Tema natra dikelola di repo terpisah di https://github.com/OpenSID/tema-natra dan ditambahkan ke OpenSID menggunakan git subtree di folder themes/natra. Perubahan yang dilakukan pada tema natra secara langsung di repo OpenSID akan tersimpan di repo OpenSID. Tetapi juga perlu dipush ke repo natra supaya mengupdate script yang ada di situ. Caranya dengan menjalankan command terminal berikut di folder utama (root):

git subtree push --prefix themes/natra https://github.com/OpenSID/tema-natra.git master

Muat ke situs https://demosid.opendesa.id/

Kita perlu selalu usahakan supaya situs demo di https://demosid.opendesa.id/ berisi pra-rilis terkini. Tujuannya supaya pengguna bisa ikut ujicoba, dan juga supaya pengguna yang berminat menggunakan perbaikan/tambahan baru dapat memeriksa dulu sebelum memutuskan apakah mereka akan update ke versi terkini.