- Langsung komentar style sebelum memahami intent.
- Approve hanya karena CI hijau.
- Request changes tanpa menjelaskan risiko.
- Mengulang opini pribadi yang tidak terkait correctness.
- Tidak mengecek test ketika behavior berubah.
- Tidak mengecek migration ketika schema berubah.

[ ] Requirement/story dipahami dan tercermin di code.
[ ] Happy path benar.
[ ] Invalid input ditangani.
[ ] Missing resource ditangani.
[ ] Duplicate request ditangani jika relevan.
[ ] State transition valid.
[ ] Transaction boundary benar.
[ ] Retry tidak menyebabkan double effect.
[ ] Error response tidak misleading.
[ ] Tidak ada silent failure.

Apa yang terjadi jika request yang sama dikirim dua kali?
Apakah transition dari QUOTED ke ORDERED selalu valid?
Apakah failure setelah DB commit tetapi sebelum event publish ditangani?
Apakah cache Redis invalidated ketika entity berubah?
Apakah timeout dependency menyebabkan response yang benar?

[ ] Apakah PR mengubah business rule?
[ ] Apakah rule tersebut berlaku di semua entry point?
[ ] Apakah validation ada di layer yang tepat?
[ ] Apakah batch/job/event consumer juga mengikuti rule yang sama?
[ ] Apakah backward compatibility data lama dipertimbangkan?
[ ] Apakah test mencakup invalid state?

// Red flag: validation hanya di resource layer,
// tetapi service method juga dipakai oleh consumer/job lain.
@POST
public Response submitQuote(SubmitQuoteRequest request) {
    validate(request);
    quoteService.submit(request.quoteId());
    return Response.ok().build();
}

Apakah invariant ini harus ditegakkan di resource layer, service layer, entity/domain layer, database constraint, atau kombinasi?

[ ] Existing endpoint tetap compatible.
[ ] Field baru bersifat optional jika consumer lama harus tetap jalan.
[ ] Field lama tidak dihapus tanpa deprecation plan.
[ ] Status code change tidak mematahkan consumer.
[ ] Error payload masih bisa diparse consumer.
[ ] Enum baru tidak mematahkan switch/case consumer.
[ ] OpenAPI/contract docs diperbarui jika digunakan.
[ ] Integration test/contract test diperbarui.

Backend menambahkan enum value baru.
Consumer lama menganggap enum exhaustive.
Consumer gagal parse atau masuk default error path.

Apakah perubahan ini additive, backward-compatible, atau breaking?
Jika breaking, siapa consumer-nya dan apa migration plan-nya?

[ ] Event schema change backward-compatible.
[ ] Field baru optional atau punya default aman.
[ ] Consumer lama tidak rusak.
[ ] Producer tidak publish event sebelum state committed.
[ ] Consumer idempotent terhadap redelivery.
[ ] Retry tidak menyebabkan duplicate side effect.
[ ] DLQ behavior diketahui.
[ ] Correlation/trace ID ikut terbawa.
[ ] Event versioning strategy jelas jika ada breaking change.

- Producer publish event setelah partial update.
- Consumer menerima event duplicate dan membuat order duplicate.
- Field baru dianggap mandatory oleh consumer baru tetapi producer lama belum mengirim.
- Retry broker menyebabkan side effect non-idempotent.
- Event order diasumsikan tetapi tidak dijamin.

Jika event ini diproses dua kali, apakah hasil akhirnya tetap benar?

[ ] Migration backward-compatible dengan versi aplikasi lama dan baru.
[ ] Tidak menghapus/rename column yang masih dipakai.
[ ] Non-null column baru punya safe rollout path.
[ ] Index creation mempertimbangkan table besar.
[ ] Data backfill aman, batchable, observable.
[ ] Migration bisa diulang atau punya guard.
[ ] Rollback/roll-forward strategy jelas.
[ ] Test migration tersedia jika framework mendukung.

Step 1: Add nullable column.
Step 2: Deploy app yang menulis column baru dan masih membaca column lama.
Step 3: Backfill data.
Step 4: Deploy app yang membaca column baru.
Step 5: Setelah aman, drop old column di release terpisah.

ALTER TABLE orders DROP COLUMN legacy_status;

Apakah versi aplikasi lama masih bisa berjalan setelah migration ini diterapkan?
Apakah versi aplikasi baru masih bisa berjalan sebelum migration ini diterapkan?

[ ] Authorization dicek di boundary yang tepat.
[ ] User tidak bisa mengakses resource tenant/account lain.
[ ] Input tidak langsung digunakan untuk path, URL, shell command, SQL, atau broker routing tanpa validation.
[ ] Sensitive data tidak masuk log/error response.
[ ] Secret tidak ditulis ke repository, env dump, shell history, atau CI log.
[ ] Dependency baru punya alasan dan dicek vulnerability/license.
[ ] GitHub Actions permission minimal.
[ ] External endpoint access aman dari SSRF jika relevan.

log.info("Payment request: {}", request); // bisa bocor PII/secret/token

Data apa yang paling sensitif dalam flow ini?
Siapa yang boleh melihat/mengubah resource ini?
Apakah log baru aman jika masuk centralized logging?

[ ] Failure path penting punya log yang cukup.
[ ] Log tidak terlalu noisy.
[ ] Correlation ID/trace ID tidak hilang.
[ ] Error log menyertakan context aman.
[ ] Metric ditambahkan untuk behavior penting jika relevan.
[ ] Alert tidak menjadi noisy karena perubahan ini.
[ ] Dashboard/runbook diperbarui jika behavior operasional berubah.
[ ] Sensitive data tidak bocor.

Jika perubahan ini gagal di production, evidence apa yang akan kita punya dalam 5 menit pertama?

[ ] Tidak ada loop yang melakukan query/call remote per item tanpa alasan.
[ ] Query baru punya index yang sesuai jika diperlukan.
[ ] Payload size masih masuk akal.
[ ] Retry punya backoff dan limit.
[ ] Cache invalidation/hit behavior dipahami.
[ ] Thread pool/connection pool tidak mudah exhausted.
[ ] Test atau benchmark tersedia untuk path kritikal jika relevan.

for (LineItem item : quote.getItems()) {
    priceRepository.findByProductId(item.productId()); // potensi N+1 query
}

Bagaimana behavior ini berubah ketika data 10x lebih besar atau consumer traffic 10x lebih tinggi?

[ ] Test baru sesuai perubahan behavior.
[ ] Test mencakup negative path.
[ ] Test mencakup boundary condition.
[ ] Test mencakup compatibility jika API/event berubah.
[ ] Test tidak hanya mengejar coverage angka.
[ ] Test tidak bergantung pada waktu/random/order tanpa kontrol.
[ ] Test failure message cukup jelas.
[ ] Flaky risk rendah.

[ ] Rollback aplikasi aman terhadap schema/data baru.
[ ] Rollback tidak mematahkan event/API consumer.
[ ] Feature flag/config bisa men-disable behavior baru jika relevan.
[ ] Data migration irreversible dijelaskan.
[ ] Release note menyebut risiko recovery.
[ ] Smoke test setelah rollback diketahui.

PR mengubah schema, data, producer event, dan consumer logic sekaligus tanpa rollout plan.

Jika deploy ini gagal 30 menit setelah release, apakah kita rollback, roll-forward, disable flag, atau run repair script?

[ ] Nama class/method/variable menggambarkan domain intent.
[ ] Logic kompleks punya struktur yang mudah diikuti.
[ ] Tidak ada abstraction yang terlalu dini.
[ ] Tidak ada duplication berbahaya.
[ ] Boundary antar layer jelas.
[ ] Error handling konsisten.
[ ] Komentar menjelaskan alasan, bukan mengulang code.
[ ] Dokumentasi diperbarui jika behavior/tooling berubah.

- issue blocking: correctness/security/release risk;
- issue important but non-blocking: naming, minor structure;
- personal preference: style yang tidak berdampak.

[ ] Dependency baru benar-benar dibutuhkan.
[ ] Version dikelola oleh dependencyManagement/BOM jika policy tim demikian.
[ ] Scope benar: compile/provided/runtime/test.
[ ] Tidak ada transitive conflict signifikan.
[ ] Exclusion punya alasan jelas.
[ ] Plugin version dipin atau dikelola oleh pluginManagement.
[ ] Build tetap reproducible.
[ ] Dependency vulnerability/license dicek.
[ ] CI command tetap konsisten dengan local command.

mvn dependency:tree
mvn dependency:tree -Dincludes=groupId:artifactId
mvn help:effective-pom
mvn -U clean verify

Apakah perubahan POM ini mengubah runtime classpath, hanya test classpath, atau hanya build behavior?

[ ] Trigger workflow sesuai kebutuhan.
[ ] Required checks tidak bisa dibypass secara tidak sengaja.
[ ] Secrets tidak terekspos ke PR tidak trusted.
[ ] Permission token minimal.
[ ] Cache key tidak menyebabkan stale dependency berbahaya.
[ ] Artifact yang dipublish traceable ke commit/tag.
[ ] Deployment gate/approval tetap terjaga.
[ ] Failure log cukup jelas.
[ ] Pipeline tidak terlalu lambat tanpa alasan.

permissions: write-all

Apakah perubahan pipeline ini bisa membuat code yang belum tervalidasi masuk release path?

[Blocking] Ini terlihat bisa mematahkan consumer lama karena field `status` berubah dari string ke object.
Bisakah kita buat perubahan ini additive dulu, misalnya menambahkan `statusDetail` dan mempertahankan `status` sampai consumer selesai migrasi?

[Suggestion] Nama `process()` agak terlalu umum untuk flow ini. `submitQuote()` mungkin lebih jelas, tapi ini tidak blocking.

[Question] Apakah consumer RabbitMQ ini idempotent saat message redelivered? Saya belum melihat guard berdasarkan message ID/order ID.

- "Ini jelek."
- "Kenapa begini?"
- "Harusnya jelas."
- "Saya lebih suka style X." tanpa dampak correctness/maintainability.

Blocking: harus diperbaiki sebelum merge.
Non-blocking: sebaiknya diperbaiki, tapi tidak menghalangi.
Question: butuh clarification.
Nit: kecil, optional.
Praise: tandai keputusan baik agar pattern tersebar.

1. Reviewer hanya fokus style dan melewatkan invariant.
2. Reviewer approve karena author senior.
3. Reviewer percaya CI hijau tanpa membaca test relevance.
4. Reviewer tidak melihat compatibility consumer.
5. Reviewer tidak mengecek migration lock/backward compatibility.
6. Reviewer mengabaikan observability.
7. Reviewer tidak bertanya rollback.
8. Reviewer memberi komentar terlalu banyak untuk hal kecil sehingga risiko besar tenggelam.
9. Reviewer tidak memahami pipeline/dependency impact.
10. Reviewer tidak meminta internal verification untuk hal yang team-specific.

- PR besar approve dalam waktu sangat singkat.
- Tidak ada komentar tentang risk pada PR high-risk.
- Tidak ada test discussion pada behavior change.
- Tidak ada migration discussion pada schema change.
- Tidak ada compatibility discussion pada API/event change.
- Review hanya formatting/naming.

Context
[ ] Saya memahami problem/intent PR.
[ ] Scope PR jelas dan tidak terlalu luas.
[ ] Linked issue/story/incidents relevan.

Correctness
[ ] Happy path benar.
[ ] Edge case penting ditangani.
[ ] Error handling konsisten.
[ ] Idempotency/retry dipertimbangkan.
[ ] Transaction boundary benar.

Domain
[ ] Domain invariant tidak rusak.
[ ] State transition valid.
[ ] Validation berada di layer yang tepat.

Compatibility
[ ] API backward-compatible atau migration plan jelas.
[ ] Event backward-compatible atau versioning plan jelas.
[ ] DB migration backward-compatible.

Security
[ ] Authorization boundary aman.
[ ] Sensitive data tidak bocor.
[ ] Dependency/workflow/secret risk dicek.

Observability
[ ] Log/metric/trace cukup untuk debugging.
[ ] Correlation ID tidak hilang.
[ ] Sensitive log redacted.

Performance
[ ] Tidak ada N+1 obvious.
[ ] Query/index/payload/retry aman.
[ ] Resource usage masuk akal.

Testing
[ ] Test menutupi risiko utama.
[ ] Failure path diuji.
[ ] Test deterministik.

Release
[ ] Rollback/roll-forward story jelas.
[ ] Feature flag/config gate ada jika relevan.
[ ] Release note/docs diperbarui jika perlu.

Maintainability
[ ] Struktur code mudah dipahami.
[ ] Naming menggambarkan intent.
[ ] Abstraction tidak berlebihan.

Decision
[ ] Komentar actionable.
[ ] Blocking vs non-blocking jelas.
[ ] Approval diberikan hanya jika risiko acceptable.

Repository and workflow
[ ] Apa branch strategy resmi?
[ ] Apa merge strategy yang digunakan: squash, rebase, atau merge commit?
[ ] Apa PR template wajib?
[ ] Apa required checks sebelum merge?
[ ] Siapa CODEOWNERS untuk area tertentu?

Domain and architecture
[ ] Apa domain invariant utama Quote & Order?
[ ] Apa state machine quote/order resmi?
[ ] Apa API compatibility policy?
[ ] Apa event schema/versioning policy?
[ ] Apa migration strategy yang diterima?

Testing and CI
[ ] Test apa yang wajib untuk behavior change?
[ ] Apakah contract/integration test tersedia?
[ ] Apakah flaky test punya process khusus?
[ ] Apakah coverage threshold digunakan?

Security and compliance
[ ] Apa secure coding checklist internal?
[ ] Apa policy dependency vulnerability?
[ ] Apa policy secret/log redaction?
[ ] Apa approval tambahan untuk security-sensitive changes?

Release and operations
[ ] Apa release checklist internal?
[ ] Apa rollback process?
[ ] Apa incident/runbook template?
[ ] Apa observability standard?
[ ] Siapa yang perlu approve perubahan release/pipeline/deployment?

1. Baca deskripsi dan linked issue.
2. Tentukan risk category: low, medium, high.
3. Cek files changed untuk surface area.
4. Review behavior utama.
5. Review test yang membuktikan behavior.
6. Review compatibility dan rollback jika production-impacting.
7. Review observability/security jika runtime-impacting.
8. Tulis komentar actionable.
9. Approve hanya jika risk acceptable.

Untuk PR high-risk, jangan review dari diff saja. Review dari lifecycle perubahan sampai production.