Background
Issue #145's fix added five new `pruneOrphanedDNSRecords(..., desired=nil)` call sites across HTTPRoute, TLSRoute, Gateway (×2), and Service source-controllers. Each call site is a ~7-line block with the same shape:
```go
pruned, perr := pruneOrphanedDNSRecords(ctx, r.Client, "", .Name, .Namespace, nil)
if perr != nil {
logger.Error(perr, "orphan-prune failed during deactivation sweep")
} else if len(pruned) > 0 {
r.dedupe.emit(r.Recorder, , corev1.EventTypeNormal, conventions.ReasonOrphanedDNSRecordPruned,
fmt.Sprintf("deleted %d orphaned DNSRecord CR(s) on source deactivation", len(pruned)))
}
```
The same shape ALSO appears in the four end-of-reconcile happy-path prune call sites (with a different `desired` value and a different message text).
Two cleanup items
1. DRY review — extract a helper?
Per DRY's `would-all-instances-need-to-change-together` criterion, the message text, event reason, event type, and log message are shared knowledge across all nine call sites. A future change to the prune-event semantics would touch all nine.
But KISS argues against premature extraction: a helper would need `ctx, client, kind, obj, dedupe, recorder, logger, desired` plus the "deactivation vs. happy-path" message switch. The signature might be nearly as long as the inlined block.
Decision needed: extract once a third "prune emits a templated event" call site appears (rule of three), OR extract now because the shared-knowledge criterion is already clear at 9 sites.
2. Rename `perr` to `pruneErr`
`perr` reads oddly and is ungrepable. `pruneErr` is more searchable. Pre-existing convention used `perr` for the four happy-path sites; the #145 fix followed suit. A sweeping rename across all nine sites would be a small, contained churn commit.
Why deferred
Both are minors from the comprehensive review of #145's fix. The design's scope was the bug fix; cleanup belongs in its own PR with its own thinking.
Surfaced by
Independent comprehensive review of the #145 fix branch (`feature/source-controller-orphan-gc`).
Background
Issue #145's fix added five new `pruneOrphanedDNSRecords(..., desired=nil)` call sites across HTTPRoute, TLSRoute, Gateway (×2), and Service source-controllers. Each call site is a ~7-line block with the same shape:
```go
pruned, perr := pruneOrphanedDNSRecords(ctx, r.Client, "", .Name, .Namespace, nil)
if perr != nil {
logger.Error(perr, "orphan-prune failed during deactivation sweep")
} else if len(pruned) > 0 {
r.dedupe.emit(r.Recorder, , corev1.EventTypeNormal, conventions.ReasonOrphanedDNSRecordPruned,
fmt.Sprintf("deleted %d orphaned DNSRecord CR(s) on source deactivation", len(pruned)))
}
```
The same shape ALSO appears in the four end-of-reconcile happy-path prune call sites (with a different `desired` value and a different message text).
Two cleanup items
1. DRY review — extract a helper?
Per DRY's `would-all-instances-need-to-change-together` criterion, the message text, event reason, event type, and log message are shared knowledge across all nine call sites. A future change to the prune-event semantics would touch all nine.
But KISS argues against premature extraction: a helper would need `ctx, client, kind, obj, dedupe, recorder, logger, desired` plus the "deactivation vs. happy-path" message switch. The signature might be nearly as long as the inlined block.
Decision needed: extract once a third "prune emits a templated event" call site appears (rule of three), OR extract now because the shared-knowledge criterion is already clear at 9 sites.
2. Rename `perr` to `pruneErr`
`perr` reads oddly and is ungrepable. `pruneErr` is more searchable. Pre-existing convention used `perr` for the four happy-path sites; the #145 fix followed suit. A sweeping rename across all nine sites would be a small, contained churn commit.
Why deferred
Both are minors from the comprehensive review of #145's fix. The design's scope was the bug fix; cleanup belongs in its own PR with its own thinking.
Surfaced by
Independent comprehensive review of the #145 fix branch (`feature/source-controller-orphan-gc`).