Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perbaikan Error Lacak Komplain #60

Merged
merged 3 commits into from
Dec 8, 2020
Merged

Conversation

dirgareborn
Copy link
Contributor

@dirgareborn dirgareborn commented Nov 29, 2020

@dirgareborn, Sepertinya PR ini untuk issue berikut:

  1. Pilihan Nama Kecamatan #46
  2. Luas wilayah tidak bisa menampung desimal. #59

Betul? Mohon konfirmasi. Mohon selalu jelaskan issue yg dikerjakan pada setiap PR.

Juga usahakan setiap PR hanya mengerjakan satu issue saja, kecuali kalau ada issue yg erat sekali keterkaitannya.

@eddieridwan
Copy link
Contributor

@dirgareborn terima kasih. Mohon buatkan issue untuk PR anda, supaya bisa diketahui apa masalah/perbaikan yg anda lakukan dari segi penggunaan/dsbnya. Issue diperlukan supaya kami dapat me-review berdasarkan issue tersebut. Terima kasih.

@dirgareborn
Copy link
Contributor Author

terima kasih atas tanggapannya pak @eddieridwan . mengenai PR ini hanya untuk issue #46 .

@@ -24,7 +24,7 @@
<label for="luas_wilayah" class="control-label col-md-4 col-sm-3 col-xs-12">Luas Wilayah (km<sup>2</sup>)<span class="required">*</span></label>

<div class="col-md-6 col-sm-6 col-xs-12">
{!! Form::number('luas_wilayah', null, ['class' => 'form-control', 'id'=>'luas_wilayah', 'required' => true, 'placeholder'=>'Luas Wilayah Desa']) !!}
{!! Form::number('luas_wilayah', null, ['class' => 'form-control', 'id'=>'luas_wilayah', 'required' => true, 'placeholder'=>'Luas Wilayah Desa','step' => '0.1']) !!}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apakah ini untuk #59?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ya betul pak. maaf untuk commit pertama belum sempat buatkan issuenya. commit ke 3 yg perbaikan issue #46

Copy link
Contributor

@eddieridwan eddieridwan Dec 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Untuk setiap PR seharusnya anda buat branch khusus untuk PR tsb (yaitu tidak menggunakan master). Kemudian yg dicommit ke PR/branch tersebut HANYA perubahan untuk PR itu saja. Sehingga tidak mencampur commit. Perubahan terkait dengan issue yg lain dicommit ke branch yg lain, sehingga masing2 PR/branch berdiri sendiri.

Dengan cara anda sekarang, semua commit anda masuk ke branch master sehingga PR ini mencampur perubahan untuk beberapa issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jadi buat satu branch/PR untuk #59. Dan buat branch/PR lain untuk #46. Cara ini dilakukan supaya masing2 PR bisa direview secara terpisah. Karena belum tentu semua solusi setiap issue siap untuk digabung -- mungkin saja solusi untuk #46 siap digabung, tetapi solusi untuk #59 masih perlu disempurnakan.

@@ -23,7 +23,7 @@
<label for="luas_wilayah" class="control-label col-md-4 col-sm-3 col-xs-12">Luas Wilayah (km<sup>2</sup>)<span class="required">*</span></label>

<div class="col-md-6 col-sm-6 col-xs-12">
{!! Form::number('luas_wilayah', null, ['class' => 'form-control', 'id'=>'luas_wilayah', 'required' => true, 'placeholder'=>'Luas Wilayah Desa']) !!}
{!! Form::number('luas_wilayah', null, ['class' => 'form-control', 'id'=>'luas_wilayah', 'required' => true, 'placeholder'=>'Luas Wilayah Desa','step' => '0.1']) !!}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apakah ini untuk #59?

@dirgareborn dirgareborn changed the title Perbaikan Error Show Tracking Komplain Perbaikan Error Lacak Komplain Dec 2, 2020
@dirgareborn
Copy link
Contributor Author

Perbaikan Error Show Tracking Komplain untuk issue #67

Comment on lines +24 to +28
if(substr($getWilayah->kode,0,2) == 91 or substr($getWilayah->kode,0,2) == 92){
$sebutan_wilayah = 'Kecamatan';
}else{
$sebutan_wilayah = 'Distrik';
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dirgareborn , sepertinya anda belum menggunakan $sebutan_wilayah ini untuk mengubah tampilan, sebagaimana diminta untuk issue #46 . Sebagai contoh di app/Http/Controllers/Dashboard/DashboardProfilController.php, profil kecamatan masih menampilkan 'Kecamatan' yg disedikan sebagai string literal. Untuk issue #46, sepertinya semua string literal Kecamatan perlu diganti dengan $sebutan_wilayah supaya bisa menampilkan Kecamatan atau Distrik sesuai dengan kode wilayah.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eddieridwan ia betul pak belum sempat ganti secara keseluruhan :D

Copy link
Contributor

@eddieridwan eddieridwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mohon lihat komentar

@eddieridwan eddieridwan merged commit 037f87b into OpenSID:master Dec 8, 2020
@eddieridwan
Copy link
Contributor

@dirgareborn terima kasih. Sdh digabung ke master dengan perubahan:

  1. Perbaiki supaya sebutan_wilayah bisa digunakan di semua controller juga. Karena sebutan wilayah juga digunakan di controller.
  2. Perbaiki syarat sebutan wilayah terbalik. Mohon lebih teliti melakukan testing.
  3. Buat contoh penggunaan sebutan_wilayah di halaman profil dashboard. Issue Pilihan Nama Kecamatan #46 belum bisa ditutup, karena masih banyak penulisan 'Kecamatan' yg perlu disesuaikan.

@eddieridwan
Copy link
Contributor

@dirgareborn PR anda berikutnya ditunggu. Mohon batasi setiap PR hanya untuk satu issue, menggunakan branch terpisah per PR. Terima kasih.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants