review-pr

name	review-pr
description	Reviews a PR against BTP Manager conventions (reconciler patterns, conditions API, state machine, test structure, etc.).

review-pr

Review a pull request with BTP Manager-specific context in mind.

Usage

/review-pr <PR number or URL>

Examples:

• /review-pr 1234
• /review-pr https://github.com/kyma-project/btp-manager/pull/1234

---

What to do

Fetch the PR diff and review it through the lens of BTP Manager conventions. Structure your review as:

Summary

One paragraph: what the PR does and whether the approach makes sense.

Issues (if any)

List only real problems — not style nits unless they violate a hard rule. For each issue:

• File:line — description of the problem
• Severity: blocking | suggestion

BTP Manager checklist

Evaluate every item below. Only print items that have a note or failure — skip items that pass cleanly. If everything passes, write a single line: BTP Manager checklist: all clear.

Use this format for printed items only:

• ⚠️ [category] — description of the note
• ❌ [category] — description of the failure

Reconciler / state machine (if controllers/ is touched):

• New state handlers follow the existing pattern: dedicated Handle*State method dispatched from Reconcile()
• Status updates go through UpdateBtpOperatorStatus() — not direct client patches
• Finalizer (operator.kyma-project.io/btp-manager) is only added/removed in the correct lifecycle phases
• Requeue intervals use the configured values (processingStateRequeueInterval, readyStateRequeueInterval) — no hardcoded time.Duration literals

Conditions (if internal/conditions/ or status conditions are touched):

• New condition reasons are defined as constants in internal/conditions/conditions.go
• Documentation comment for each new reason is present — make test-docs would pass
• Reasons map to the correct state (Ready/Processing/Error/Warning/Deleting)

Resource management (if module-resources/ or manifest handling is touched):

• Resources to apply go in module-resources/apply/, resources to delete go in module-resources/delete/
• No direct client.Apply / client.Delete calls that bypass the manifest reconciliation path

Tests:

• Tests use Ginkgo v2 + Gomega (not testify)
• New controller tests are placed in the appropriately named file (btpoperator_controller_<concern>_test.go)
• Shared setup uses controllers/suite_test.go — no duplicate envtest bootstrapping
• make test would pass (envtest env vars sourced from scripts/testing/set-env-vars.sh)

Code generation (if api/v1alpha1/ or controller markers are touched):

• make generate and make manifests were run — generated files (zz_generated.deepcopy.go, CRD YAMLs) are up to date

Documentation:

• CLAUDE.md updated if project structure, conventions, or build/test workflow changed

General:

• No speculative abstractions or features beyond the PR scope
• Imports cleaned up (no unused imports left from changes)
• No backwards-compatibility shims for removed code

Verdict

Approve / Request changes / Comment — with one sentence explaining why.

---

Tone

• Be direct and specific. Point to file and line numbers.
• Don't praise code that is merely correct — save positive comments for non-obvious good decisions.
• Don't suggest improvements outside the PR scope.