Skip to content

Add contribution guidelines for new controllers - #959

Open
mandre wants to merge 2 commits into
k-orc:mainfrom
shiftstack:new-controller-contribution-guidelines
Open

mandre wants to merge 2 commits into
k-orc:mainfrom
shiftstack:new-controller-contribution-guidelines

Conversation

@mandre

@mandre mandre commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

Document the expected commit structure, incremental PR strategy, and deferred mutability approach for new controller contributions. Update scaffolding docs to separate the scaffolding and generated code commits.

Document the expected commit structure, incremental PR strategy, and
deferred mutability approach for new controller contributions. Update
scaffolding docs to separate the scaffolding and generated code commits.
@github-actions github-actions Bot added the semver:patch No API change label Oct 2, 2026
Ensure a tracking issue exists before submitting a
bug fix PR, so it can be referenced in commit messages,
changelogs, and future discussions.

@winiciusallan winiciusallan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally agree with the issue thing. Besides, I'd like to suggest two more things:

  1. AI guidelines if we want to have one (this helps to define the way we handle AI-generated code and avoid slop);
  2. Add to our review skill to "checkbox" these points.

Comment thread AGENTS.md
Always structure new controller branches as multi-commit, separating generated
code from hand-written code:

1. **Scaffolding commit**: Raw output of `go run ./cmd/scaffold-controller`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth adding an example commit?

Comment thread AGENTS.md
- **Multiple reconcilers**: `internal/controllers/trunk/` - `updateResource` + `reconcileSubports` + tags
- **Complex**: `internal/controllers/server/` - Multiple dependencies, many reconcilers

## Contributing New Controllers

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can reference this same section from CONTRIBUTING.md, making this file shorter.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver:patch No API change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants