docs: add comprehensive code review standards from 10 PRs

[jhjaggars]

Summary

Add comprehensive coding standards to CONTRIBUTING.md based on maintainer feedback from 10 highly-reviewed PRs, capturing patterns from 8+ maintainers.

Changes

New Sections in CONTRIBUTING.md

1. Controller Code Review Standards (existing, enhanced)

   • Error handling (retryable vs non-retryable)
   • Early filtering by namespace labels
   • Reference validation before fetching
   • Conditional patching
   • Logging conventions (ctrl.LoggerFrom vs klog)
   • Events for user-visible changes
   • Multi-tenancy awareness

2. API Design Standards (NEW)

   • Use enums instead of bools (bools don't evolve well)
   • Include units in duration field names (delayAfterAddSeconds)
   • Document limits and defaults in godoc
   • Default in code, not API level
   • Set MinItems=1 to prevent empty list issues
   • Mark immutable fields with XValidation
   • Use pointers for optional fields with valid empty values

3. Status Update Patterns (NEW)

   • Use Patch instead of Update for status updates
   • Use meta.SetStatusCondition() return value instead of DeepEqual
   • Define condition constants together

4. Code Organization (NEW)

   • Extract complex logic to helper functions
   • Keep related constants together

5. Breaking Changes (NEW)

   • Immutable field changes require migration
   • Cannot add required fields to shipped APIs
   • Document behavioral changes in release notes

6. Naming and Documentation (NEW)

   • API vs Api naming (kubeAPICustomName not kubeApiCustomName)
   • Write godoc in prose, not bullet points
   • Document day 1 and day 2 behavior

7. Condition Patterns (NEW)

   • Conditions should be informative, not blocking
   • Use Unknown for error states, not removal
   • Set conditions consistently for all error paths
   • Be specific in condition documentation

8. Testing (NEW)

   • Unit test validation functions
   • Tests should be deterministic

9. Performance (NEW)

   • Reuse existing clients
   • Be careful with requests to guest cluster

Updated .coderabbit.yaml

Added path-specific review instructions:

• **/controllers/**/*.go - Enhanced with status update, condition, and performance patterns
• api/**/*.go - NEW: API design standards, naming, documentation, breaking changes
• cmd/**/*.go - Feature flags and variable naming

Source PRs Analyzed

PR	Comments	Reviewers	Key Patterns
#6975	30	csrwng	Controller error handling, logging, filtering
#6745	116	csrwng, gaol, jparrill	Breaking changes, complex logic extraction
#7260	45	jparrill, muraee, sdminonne	Status updates, Patch vs Update
#7285	24	csrwng, muraee, apahim	API pointer semantics
#6236	51	enxebre, JoelSpeed, celebdor	Enums vs bools, duration units, godoc
#7278	37	jparrill, muraee, linoyaslan	Immutable fields, defaults in code
#5458	88	enxebre, csrwng, muraee	Naming conventions, unit testing
#6703	44	bryan-cox, devguyio	Unreleased API changes, DRY
#5931	61	enxebre, bryan-cox	Conditions: informative not blocking
#4538	47	JoelSpeed, enxebre	Required field breaks, client reuse

Benefits

For Contributors

• Self-service review - Check your code against standards before submitting
• Faster PR cycles - Catch common issues early
• Learn from maintainers - See patterns from experienced reviewers
• Clear examples - Good vs bad code patterns with explanations

For Maintainers

• Consistent feedback - Same standards applied across reviews
• Reduced repetition - Don't explain the same patterns repeatedly
• CodeRabbit automation - AI reviewer enforces standards automatically
• Institutional knowledge - Captures review patterns in documentation

For CodeRabbit

• Path-specific rules - Different standards for controllers vs API vs CLI code
• Automated enforcement - Catches issues before human review
• References CONTRIBUTING.md - Single source of truth

Test Plan

• Verify CONTRIBUTING.md renders correctly on GitHub
• CodeRabbit configuration is valid YAML
• CodeRabbit reviews this PR and applies the standards
• Gather feedback from maintainers on coverage and accuracy

Stats

• 453 lines of standards documentation added
• 9 new sections covering controllers, APIs, conditions, testing, performance
• 10 PRs analyzed (900+ total comments)
• 8+ maintainers represented

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[openshift-ci]

Please specify an area label

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[muraee]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhjaggars, muraee

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [muraee]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[muraee]

@coderabbitai configuration

[coderabbitai]

CodeRabbit configuration

language: en-US
tone_instructions: ''
early_access: false
enable_free_tier: true
inheritance: false
reviews:
  profile: chill
  request_changes_workflow: false
  high_level_summary: true
  high_level_summary_instructions: ''
  high_level_summary_placeholder: '@coderabbitai summary'
  high_level_summary_in_walkthrough: false
  auto_title_placeholder: '@coderabbitai'
  auto_title_instructions: ''
  review_status: true
  commit_status: true
  fail_commit_status: false
  collapse_walkthrough: false
  changed_files_summary: true
  sequence_diagrams: true
  estimate_code_review_effort: true
  assess_linked_issues: true
  related_issues: true
  related_prs: true
  suggested_labels: true
  labeling_instructions: []
  auto_apply_labels: false
  suggested_reviewers: true
  auto_assign_reviewers: false
  in_progress_fortune: true
  poem: true
  enable_prompt_for_ai_agents: true
  path_filters: []
  path_instructions:
    - path: '**/controllers/**/*.go'
      instructions: >
        Follow .github/CONTRIBUTING.md "Controller Code Review Standards"
        section.

        Key checks:

        - Non-retryable conditions (missing labels, wrong resource type) should
        return nil, not error

        - Check namespace labels
        (hypershift.openshift.io/hosted-control-plane=true) early

        - Validate references (non-nil, correct Kind) before fetching

        - Only patch when there's an actual difference

        - Use ctrl.LoggerFrom(ctx), not klog

        - Use Events for user-visible state changes

        - Use Patch instead of Update for status updates

        - Use meta.SetStatusCondition() return value instead of DeepEqual

        - Extract complex anonymous functions to helper functions

        - Conditions should be informative, not blocking reconciliation

        - Use Unknown for error states, not removal

        - Reuse existing clients, don't recreate
    - path: api/**/*.go
      instructions: >
        Follow .github/CONTRIBUTING.md "API Design Standards" section.

        Key checks:

        - Use enums instead of bools (bools don't evolve well)

        - For durations, include unit in field name (e.g., delayAfterAddSeconds)

        - Document limits and defaults in godoc (users can't see validations)

        - Default in code, not API level

        - Set MinItems=1 to prevent empty list issues

        - Mark immutable fields with XValidation

        - Use pointers for optional fields where empty string is valid

        - Define condition constants in same file as condition types

        - Use API not Api in field names (kubeAPICustomName not
        kubeApiCustomName)

        - Write godoc in prose, not bullet points

        - Document day 1 and day 2 behavior

        - Cannot add required fields to shipped APIs (breaking change)
    - path: cmd/**/*.go
      instructions: >
        Follow .github/CONTRIBUTING.md "Controller Code Review Standards"
        section.

        Key checks:

        - Feature flags should require all necessary config

        - Clear variable names that reflect purpose
  abort_on_close: true
  disable_cache: false
  auto_review:
    enabled: true
    auto_incremental_review: true
    ignore_title_keywords: []
    labels: []
    drafts: false
    base_branches: []
    ignore_usernames: []
  finishing_touches:
    docstrings:
      enabled: true
    unit_tests:
      enabled: true
  pre_merge_checks:
    docstrings:
      mode: warning
      threshold: 80
    title:
      mode: warning
      requirements: ''
    description:
      mode: warning
    issue_assessment:
      mode: warning
    custom_checks: []
  tools:
    ast-grep:
      rule_dirs: []
      util_dirs: []
      essential_rules: true
      packages: []
    shellcheck:
      enabled: true
    ruff:
      enabled: true
    markdownlint:
      enabled: true
    github-checks:
      enabled: true
      timeout_ms: 90000
    languagetool:
      enabled: true
      enabled_rules: []
      disabled_rules: []
      enabled_categories: []
      disabled_categories: []
      enabled_only: false
      level: default
    biome:
      enabled: true
    hadolint:
      enabled: true
    swiftlint:
      enabled: true
    phpstan:
      enabled: true
      level: default
    phpmd:
      enabled: true
    phpcs:
      enabled: true
    golangci-lint:
      enabled: true
    yamllint:
      enabled: true
    gitleaks:
      enabled: true
    checkov:
      enabled: true
    detekt:
      enabled: true
    eslint:
      enabled: true
    flake8:
      enabled: true
    fortitudeLint:
      enabled: true
    rubocop:
      enabled: true
    buf:
      enabled: true
    regal:
      enabled: true
    actionlint:
      enabled: true
    pmd:
      enabled: true
    clang:
      enabled: true
    cppcheck:
      enabled: true
    semgrep:
      enabled: true
    circleci:
      enabled: true
    clippy:
      enabled: true
    sqlfluff:
      enabled: true
    prismaLint:
      enabled: true
    pylint:
      enabled: true
    oxc:
      enabled: true
    shopifyThemeCheck:
      enabled: true
    luacheck:
      enabled: true
    brakeman:
      enabled: true
    dotenvLint:
      enabled: true
    htmlhint:
      enabled: true
    checkmake:
      enabled: true
    osvScanner:
      enabled: true
chat:
  art: true
  auto_reply: true
  integrations:
    jira:
      usage: auto
    linear:
      usage: auto
knowledge_base:
  opt_out: false
  web_search:
    enabled: true
  code_guidelines:
    enabled: true
    filePatterns: []
  learnings:
    scope: auto
  issues:
    scope: auto
  jira:
    usage: auto
    project_keys: []
  linear:
    usage: auto
    team_keys: []
  pull_requests:
    scope: auto
  mcp:
    usage: auto
    disabled_servers: []
code_generation:
  docstrings:
    language: en-US
    path_instructions: []
  unit_tests:
    path_instructions: []
issue_enrichment:
  auto_enrich:
    enabled: true
  planning:
    enabled: true
    auto_planning:
      enabled: true
      labels: []
  labeling:
    labeling_instructions: []
    auto_apply_labels: false

[bryan-cox]

Rather than this section, shouldn't we just link to the OpenShift API Standards?