Skip to content

Instantly share code, notes, and snippets.

@negz
Last active September 12, 2025 04:49
Show Gist options
  • Save negz/f67a26e4e88ef0a3225083ab415e1211 to your computer and use it in GitHub Desktop.
Save negz/f67a26e4e88ef0a3225083ab415e1211 to your computer and use it in GitHub Desktop.
PR Style Guide for LLM Agents

PR Style Guide for LLM Agents

Git Commit and PR Style Guide

This guide helps LLM agents write git commits and pull requests in Nic Cope's style.

Git Commit Messages

Required Format

  • Always include: Signed-off-by: Nic Cope <[email protected]>
  • Subject line: 50-80 characters, imperative mood
  • Body: Wrap at 80 characters, explain the "why"

Subject Line Patterns

  • 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"

Body Content

  • 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"

Example Structure

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]>

Pull Request Descriptions

Required Elements

  • 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

Content Structure

  1. Issue references (first line)
  2. Problem statement (what's broken/missing)
  3. Solution summary (technical approach)
  4. Implementation details (architecture, trade-offs)
  5. Testing evidence (command outputs, examples)
  6. Migration notes (if breaking changes)

Writing Style

  • 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

What NOT to Do

  • 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

Example Opening

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.

Testing Evidence Format

Include concrete before/after examples:

Before:

$ kubectl get crd | grep providerconfig
# No output - ProviderConfig CRDs were converted to inactive MRDs

After:

$ 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

Checklist Guidelines

  • Check items that apply: - [x] Item
  • Strike through irrelevant items: - [ ] ~Item~
  • Be honest about what was and wasn't done

Architecture Explanations

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

Example Complex Description

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.

Respectful Communication

  • 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

Key Principles for LLMs

  1. Be technical, not promotional
  2. Explain problems before solutions
  3. Include concrete evidence
  4. Acknowledge complexity honestly
  5. Focus on "why", not just "what"
  6. Never brag or oversell
  7. Don't state the obvious
  8. Use precise, domain-specific terminology
  9. Show, don't just tell
  10. Direct reviewer attention proactively
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment