PR Style Guide for LLM Agents
This guide helps LLM agents write git commits and pull requests in Nic Cope's style.
- Always include:
Signed-off-by: Nic Cope <[email protected]> - Subject line: 50-80 characters, imperative mood
- Body: Wrap at 80 characters, explain the "why"
- Start with imperative verbs: "Fix", "Add", "Update", "Implement", "Use", "Support"
- Be specific and technical: "Fix CRD-to-MRD converter to preserve provider configuration CRDs"
- Include scope when helpful: "Use WIRE_JSON level buf breaking change detection"
- Avoid vague terms: Not "Update code" but "Fix composed resource names containing invalid characters"
- Problem-first: Explain what was wrong or missing
- Solution flow: Use natural language like "This commit replaces..." or "This change updates..." not bare imperatives
- Technical details: Include implementation specifics
- Behavior changes: Show before/after examples when relevant
- Context: Reference related issues, design decisions, trade-offs
- No bragging: Don't say things are "elegant", "robust", or "powerful"
Fix XRD controller restart to detect all spec changes
When XRD spec fields change, the XR controller doesn't always restart
automatically. Users must manually restart the Crossplane deployment for
some changes to take effect, as reported in issue #6736.
The existing restart logic only detected referenceable version changes.
This missed other spec changes like connection details modifications.
This commit replaces the referenceable version based detection with
generation-based controller restart detection using the existing
condition system. The ControllerNeedsRestart() helper checks if the
WatchingComposite condition's observedGeneration differs from the
current metadata.generation.
Fixes #6736.
Signed-off-by: Nic Cope <[email protected]>
- Use standard Crossplane PR template
- Fill checklist completely, strike through irrelevant items with
~text~ - Start with issue references: "Fixes #XXXX" or "Closes #XXXX"
- Lead with problem statement, then solution
- Issue references (first line)
- Problem statement (what's broken/missing)
- Solution summary (technical approach)
- Implementation details (architecture, trade-offs)
- Testing evidence (command outputs, examples)
- Migration notes (if breaking changes)
- Technical and precise: Use exact terminology
- Problem-focused: Always explain the underlying issue
- Honest about complexity: Acknowledge hacks, limitations, edge cases
- Proactive: Direct reviewer attention to complex areas
- Evidence-based: Include concrete examples and test outputs
- No bragging: Don't describe changes as "elegant", "robust", "powerful", etc.
- No table stakes: Don't explicitly mention that tests were added/updated unless noteworthy
- No marketing speak: Avoid "This exciting new feature" or similar language
- No obvious statements: Don't say "This PR adds X" when the title already says it
Fixes #6719
The CRD-to-MRD converter was converting all CRDs to MRDs, including provider
configuration types like `ProviderConfig`, `ClusterProviderConfig`, and
`ProviderConfigUsage`. These are not managed resources and should remain as
regular CRDs that get installed immediately.
This PR updates the converter to use `isManagedResource()` to identify which
CRDs represent actual managed resources. Provider configuration types are
excluded from conversion and remain as regular CRDs in the output.
Include concrete before/after examples:
Before:
$ kubectl get crd | grep providerconfig
# No output - ProviderConfig CRDs were converted to inactive MRDsAfter:
$ kubectl get crd | grep providerconfig
clusterproviderconfigs.aws.m.upbound.io 2025-08-12T19:41:45Z
providerconfigs.aws.m.upbound.io 2025-08-12T19:41:45Z- Check items that apply:
- [x] Item - Strike through irrelevant items:
- [ ] ~Item~ - Be honest about what was and wasn't done
When describing complex changes, include:
- How the change fits into existing systems
- What alternatives were considered
- Why this approach was chosen
- Any limitations or trade-offs
The `WatchOperation` controller uses `engine.Start()` to dynamically create
new controller instances at runtime, then `engine.StartWatches()` to make them
watch arbitrary resource types. Each `WatchOperation` gets its own isolated
controller with a unique name like `watched/my-watch-op`. When the
`WatchOperation` is deleted, we call `engine.Stop()` to clean up the dynamic
controller. This reuses the same engine infrastructure that powers composition
functions.
- Focus on what's different/improved, never criticize previous implementations
- Use neutral language: "The converter now distinguishes..." not "The old converter was broken"
- Remember someone worked hard on the existing code
- Be technical, not promotional
- Explain problems before solutions
- Include concrete evidence
- Acknowledge complexity honestly
- Focus on "why", not just "what"
- Never brag or oversell
- Don't state the obvious
- Use precise, domain-specific terminology
- Show, don't just tell
- Direct reviewer attention proactively