Skip to Content
Engineering HandbookHow We Review PRs

How We Review PRs

Review itu buat naikin kualitas + sebar konteks, bukan gerbang ego. Halaman ini soal cara review. Buat lifecycle, merge order, dan risk class lihat Engineering Process.

Author — sebelum minta review

Bikin PR gampang di-review, itu tugas author:

  • Kecil & fokus. Satu concern per PR. PR gede = review lambat + bug kelewat. Prefer beberapa PR kecil daripada satu raksasa.
  • Isi semua section template (## Summary, compliance checklist, ## Test Evidence). PR tanpa test evidence = belum siap.
  • Self-review dulu. Baca diff sendiri sebelum assign. Buang console.log, kode mati, file kepencet ikut.
  • Jalanin required commands lokal (lihat Process) dan tempel hasilnya. “Tests pass” tanpa bukti nggak dihitung.
  • Tulis rollback steps dan flag risiko (auth/RBAC, migration, kontrak API) eksplisit di body.

Reviewer — yang dicek

Prioritas dari yang paling penting:

  1. Correctness & intent — kode ngelakuin yang diklaim PR? Test-nya beneran nguji kenapa behavior penting, bukan cuma apa (test yang nggak bisa gagal pas logic berubah = salah).
  2. Security & authz — enforce di action, bukan cuma route. Tenant/company scoping di query & mutation. Nggak ada secret / PII di source & log.
  3. Kontrak API — endpoint/shape/auth berubah? Wajib ada “API contract impact”
    • konsumer (web & mobile) dikabarin.
  4. Migration — ada migration reviewed + rollback + test evidence? Verify di clean DB. Lihat Data & Migrations.
  5. State & konvensi — loading/empty/error/success di-handle; reuse UI (@haer/ui → app-base); token semantik, bukan literal; match konvensi lokal.

Jangan approve kalau ada test yang di-skip / di-lemahin biar hijau.

Etika & turnaround

  • Review cepat. PR nyangkut nge-block orang. Target: sekali lihat di hari kerja yang sama.
  • Komentar yang jelas & baik. Kasih alasan atau contoh. Bedain blocker (harus dibenerin) dari nit (opsional — tandain nit:).
  • Jangan bikin review jadi ping-pong. Kalau bolak-balik >2 putaran soal hal yang sama, ngobrol langsung.
  • Assume good intent dua arah. Serang masalahnya, bukan orangnya.

Approval bar

  • Minimal satu approval + semua check hijau (lint / test / build / danger / CodeQL kalau ada) sebelum merge.
  • Path sensitif — auth / rbac / permission / security, file migration, governance (AGENTS.md, .github/**, dll) — butuh Tech Lead via CODEOWNERS.
  • Author yang merge (kecuali sepakat lain). Jangan force-push branch yang di-protect. Squash/rebase sesuai norma repo.

Source of truth: AGENTS.md tiap repo + docs/engineering-development-standard.md.

Engineering ProcessTechnical Standards