Revert "Add support for Orphan management policy"#929
Conversation
📝 WalkthroughWalkthroughRemoves the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 error)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/reconciler/managed/policies.go (1)
126-126: Pre-existing typo: "No Crate" → "No Create".Tiny nit — not introduced by this PR, but noticed while reviewing. Feel free to fix if you're touching this area anyway.
📝 Suggested fix
- // No Crate and no Delete. Just update/patch the external resource. + // No Create and no Delete. Just update/patch the external resource.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/reconciler/managed/policies.go` at line 126, Fix the typo in the inline comment that reads "No Crate and no Delete. Just update/patch the external resource." — change "No Crate" to "No Create" in pkg/reconciler/managed/policies.go (look for that exact comment near the policy logic where update/patch behavior is described) so the comment correctly reads "No Create and no Delete. Just update/patch the external resource."
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/reconciler/managed/reconciler_legacy_test.go`:
- Line 89: Rename the mistyped test case key
"UnpublishConnectionDetailsDeletionPolicyDeleteOrpahn" to
"UnpublishConnectionDetailsDeletionPolicyDeleteOrphan" in the test map so the
name is spelled correctly; update the key wherever referenced in
reconciler_legacy_test.go (look for the exact string
"UnpublishConnectionDetailsDeletionPolicyDeleteOrpahn") and ensure any
assertions or lookups that use that key are updated to the corrected
"UnpublishConnectionDetailsDeletionPolicyDeleteOrphan".
---
Nitpick comments:
In `@pkg/reconciler/managed/policies.go`:
- Line 126: Fix the typo in the inline comment that reads "No Crate and no
Delete. Just update/patch the external resource." — change "No Crate" to "No
Create" in pkg/reconciler/managed/policies.go (look for that exact comment near
the policy logic where update/patch behavior is described) so the comment
correctly reads "No Create and no Delete. Just update/patch the external
resource."
| want: want{result: reconcile.Result{}}, | ||
| }, | ||
| "UnpublishConnectionDetailsDeletionPolicyDeleteOrphan": { | ||
| "UnpublishConnectionDetailsDeletionPolicyDeleteOrpahn": { |
There was a problem hiding this comment.
Typo in test case name: Orphahn → Orphan.
It looks like the revert introduced a small typo in this test case key. Could you fix this? 🙂
🔤 Suggested fix
- "UnpublishConnectionDetailsDeletionPolicyDeleteOrpahn": {
+ "UnpublishConnectionDetailsDeletionPolicyDeleteOrphan": {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "UnpublishConnectionDetailsDeletionPolicyDeleteOrpahn": { | |
| "UnpublishConnectionDetailsDeletionPolicyDeleteOrphan": { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/reconciler/managed/reconciler_legacy_test.go` at line 89, Rename the
mistyped test case key "UnpublishConnectionDetailsDeletionPolicyDeleteOrpahn" to
"UnpublishConnectionDetailsDeletionPolicyDeleteOrphan" in the test map so the
name is spelled correctly; update the key wherever referenced in
reconciler_legacy_test.go (look for the exact string
"UnpublishConnectionDetailsDeletionPolicyDeleteOrpahn") and ensure any
assertions or lookups that use that key are updated to the corrected
"UnpublishConnectionDetailsDeletionPolicyDeleteOrphan".
Reverts #864
Discussed with @bobh66 in Crossplane Slack. We would like to hold off on changes to management policies until we find a better aligned solution.
Ref: #930