Add CONTRIBUTING.md, AGENTS.md#299
Open
Jakob-Naucke wants to merge 1 commit into
Open
Conversation
Jakob-Naucke
requested review from
SpaceFace02,
alicefr,
iroykaufman,
travier,
uril and
yairpod
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Jakob-Naucke The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's GuideAdds project contribution and agent-usage guidelines, wires them into the repo, and refines the README repository structure section to better describe Rust-based operator components and supporting crates. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Jakob-Naucke
commented
Jun 26, 2026
| .await?; | ||
| ``` | ||
| - When functionality is changed, perform `make test` and [integration test](./tests/README.md) at least with basic attestation. | ||
| - Prepend your commit subject with a short focus area. Omit this when making general operator changes. Examples are `tests`, `tests/azure`, `rvs` (reference values) |
Member
Author
There was a problem hiding this comment.
Open to fix, feat also being a prefix, but IMO it should still mention the focus area (when it isn't all-operator), e.g. fix:tests: …
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The README anchor link in AGENTS.md (
./README.md##trusted-execution-cluster-operator-trusted-cluster-operator) looks malformed (double#); consider correcting it to the actual section ID so navigation works. - In AGENTS.md, the line
and agents should suggest when they detectis a sentence fragment; consider rephrasing or completing it to make the guidance clearer. - The new
.cursor/rules/project.mdcfile is added but empty in the diff; consider either removing it from the repo or populating it with the intended rules to avoid committing an unused config stub.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The README anchor link in AGENTS.md (`./README.md##trusted-execution-cluster-operator-trusted-cluster-operator`) looks malformed (double `#`); consider correcting it to the actual section ID so navigation works.
- In AGENTS.md, the line `and agents should suggest when they detect` is a sentence fragment; consider rephrasing or completing it to make the guidance clearer.
- The new `.cursor/rules/project.mdc` file is added but empty in the diff; consider either removing it from the repo or populating it with the intended rules to avoid committing an unused config stub.
## Individual Comments
### Comment 1
<location path="README.md" line_range="22-24" />
<code_context>
+### Other crates
+
- `/lib`: Shared Rust definitions, including translated CRDs
+- `/tests`: Non-unit tests, most rely on a cluster being available. Many of those also rely on a virtualization backend.
+- `/test_utils`: Operator setup helpers and virtualization backends
+
</code_context>
<issue_to_address>
**suggestion (typo):** Consider fixing the comma splice in the `/tests` description for grammatical correctness.
The sentence "Non-unit tests, most rely on a cluster being available." is a comma splice. Consider revising to "Non-unit tests; most rely on a cluster being available." or "Non-unit tests. Most rely on a cluster being available."
```suggestion
### Other crates
- `/lib`: Shared Rust definitions, including translated CRDs
- `/tests`: Non-unit tests; most rely on a cluster being available. Many of those also rely on a virtualization backend.
- `/test_utils`: Operator setup helpers and virtualization backends
```
</issue_to_address>
### Comment 2
<location path="AGENTS.md" line_range="3" />
<code_context>
+# Project objective
+
+See [README](./README.md##trusted-execution-cluster-operator-trusted-cluster-operator).
+
+- **Success looks like**: With the operator deployed, all nodes in the cluster are attested from hardware to software.
</code_context>
<issue_to_address>
**issue (bug_risk):** The README link fragment appears to have an extra `#`, which may break the anchor.
The link path uses `README.md##...`, but section anchors should use a single `#` (e.g. `README.md#section-name`). Please adjust the fragment so it matches the actual heading and the link resolves correctly.
</issue_to_address>
### Comment 3
<location path="CONTRIBUTING.md" line_range="25" />
<code_context>
+)
+.await?;
+```
+- When functionality is changed, perform `make test` and [integration test](./tests/README.md) at least with basic attestation.
+- Prepend your commit subject with a short focus area. Omit this when making general operator changes. Examples are `tests`, `tests/azure`, `rvs` (reference values)
</code_context>
<issue_to_address>
**suggestion (typo):** The phrase "and [integration test]" is missing an article and reads slightly ungrammatically.
You could rephrase to "perform `make test` and an [integration test]" or "perform `make test` and the [integration tests]" so the sentence is grammatically complete.
```suggestion
- When functionality is changed, perform `make test` and an [integration test](./tests/README.md) at least with basic attestation.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Jakob-Naucke
force-pushed
the
agents
branch
2 times, most recently
from
58e7b53 to
67973eb
Compare
SpaceFace02
reviewed
Jun 29, 2026
| @@ -0,0 +1,27 @@ | |||
| - [Sign off commits](https://git-scm.com/docs/git-commit#Documentation/git-commit.txt---signoff) | |||
Member
There was a problem hiding this comment.
Few suggestions (subject to improvement and change)
- Put an empty line between subject and body of commit message, to allow git to format
git log --oneline <branch>correctly. - Use seperate commits for dependency updates?
- Commit messages should be concise on what we doing, not why/how we are doing.
- Avoid hardcoding and pinning versions (or any item which needs to be updated often in subsequent commits) in inline comments to avoid drift.
- Be sure to wait for integration tests entire CI pipeline to pass before merging (subject to stricter gating).
Contributor
There was a problem hiding this comment.
- commit messages should include "why"
and links to those from some tools we use, and some README updates with it Signed-off-by: Jakob Naucke <jnaucke@redhat.com>
Jakob-Naucke
commented
Jun 30, 2026
| # Conventions | ||
|
|
||
| - See [CONTRIBUTING](./CONTRIBUTING.md). | ||
| - Use `Assisted-by:` or `Generated-by`: in commit messages for AI-supported contributions. |
Member
Author
There was a problem hiding this comment.
suggestion from @alicefr: align with https://github.com/kubernetes/community/blob/main/contributors/guide/pull-requests.md#ai-guidance
Contributor
There was a problem hiding this comment.
I think the pull-requests.md#ai-guidance paragraph is good, except for this sentence that does not apply to our project:
"Listing AI tooling as a co-author, co-signing commits using an AI tool, or using the assisted-by, co-developed or similar commit trailer is not allowed."
Furthermore, adding ":" is a disclosure that "This PR was written in part with the assistance of generative AI"
alicefr
reviewed
Jun 30, 2026
| - Most subjects should start with a verb in infinitive form, e.g. `Add reference value removal test` | ||
| - Put separate changes in separate commits, but bisects should stay intact | ||
| - Linting should pass, so a new definition must be used in the same commit. | ||
| - When a change requires a change to a test, the changes should be in the same commit. On the contrary, a larger new test can be in a separate commit for easier review. |
Contributor
There was a problem hiding this comment.
Do we want also that we should put a commit body with a rationale of what the commit does and why.
Contributor
|
Do we want to add a testing part? If you add a new feature or find a bug, ideally it should at least be covered by unit test |
Contributor
|
We could also mention in the contributing that user can get familiar with the getting started guide. |
Contributor
|
We could also add a new document/or a skill with the coding convention. It could help newcomers and AI agent to review the code.
|
Contributor
|
Along side the rust coding convention, it is also important to mention the api conventions for kubernetes. Not for this PR but in bootc-operator we have create a skill for reviewing api changes, see here |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
and links to those from some tools we use, and some README updates with it
Summary by Sourcery
Document contribution guidelines and agent usage conventions and link them from the main project documentation.
Documentation:
Chores: