Skip to content

Commit d0e0d83

Browse files
Merge pull request #1360 from openshift/add-auto-generated-coderabbit-rules
NO-ISSUE: Add CodeRabbit config with review rules from PR analysis
2 parents e1832cc + 77a149a commit d0e0d83

2 files changed

Lines changed: 100 additions & 0 deletions

File tree

.coderabbit.yaml

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
inheritance: true
2+
language: en-US
3+
early_access: false
4+
reviews:
5+
path_filters:
6+
- "!vendor/**"
7+
- "!go.sum"
8+
profile: chill
9+
request_changes_workflow: false
10+
high_level_summary: true
11+
poem: false
12+
review_status: true
13+
collapse_walkthrough: false
14+
path_instructions:
15+
- path: "install/**prometheusrule*"
16+
instructions: |
17+
PrometheusRule alert expressions should:
18+
- Use `last_over_time(metric{...}[5m])` to prevent failed scrapes from
19+
resetting alerts
20+
- Prefer `!= 0` for alert expressions where 0 is the happy case
21+
- Use singular metric names (e.g., `_condition` not `_conditions`) per
22+
kube-state-metrics conventions
23+
- Ensure the `for` duration is appropriate for the alert severity
24+
- Verify runbook URLs point to the correct component (not another operator)
25+
- Include a meaningful description and summary annotation
26+
- path: "install/**"
27+
instructions: |
28+
Manifest file and resource naming conventions:
29+
- Resource names should use the full component name (e.g.,
30+
`cluster-version-operator`) not acronyms (e.g., `cvo`) and should not
31+
echo the Kind (e.g., no `-sa` suffix for ServiceAccounts)
32+
- File numbering should leave gaps for future additions; avoid `ZZ` or
33+
letter suffixes for ordering — renumber sibling files instead
34+
- All manifests must have appropriate cluster-profile annotations
35+
(include.release.openshift.io/self-managed-high-availability, etc.)
36+
- Use kubernetes.io/description annotations to explain the resource's purpose
37+
- When adding new manifests, ensure the run-level ordering is correct
38+
(e.g., ServiceAccounts before RoleBindings that reference them)
39+
- path: "pkg/**/*.go"
40+
instructions: |
41+
In reconciliation and controller code, state mutations (struct field
42+
assignments, status updates) should happen as close as possible to where
43+
the value is consumed. Avoid setting fields early in a function when they
44+
are used much later — this creates risk of inconsistency if future code
45+
adds early returns or error paths between the mutation and use.
46+
47+
When sorting or deduplicating collections, ensure stable ordering by
48+
including a tiebreaker field (e.g., sort by version then by name).
49+
50+
When the same boolean condition is used for both logging and a
51+
return/branch decision, extract it to a named variable to keep logic
52+
coupled and avoid divergence if one usage is updated but not the other.
53+
54+
When modifying code, check that nearby comments,
55+
kubernetes.io/description annotations, and doc strings still accurately
56+
describe the new behavior. Outdated documentation is worse than no
57+
documentation.
58+
- path: "pkg/cvo/updatepayload.go"
59+
instructions: |
60+
When modifying container security contexts, ensure init containers are
61+
also reviewed. All containers should have `readOnlyRootFilesystem: true`
62+
unless they explicitly need to write to the filesystem. If an init
63+
container needs write access, document why in the commit message and
64+
consider using `cp` instead of `mv` to keep the source read-only.
65+
- path: "**/*_test.go"
66+
instructions: |
67+
Test code conventions:
68+
- Prefer table-driven tests over multiple similar test functions. If two
69+
test functions differ only in setup values, collapse them into one
70+
function with test-case tuples
71+
- Don't re-fetch resources already obtained in BeforeEach/setup
72+
- Don't introduce single-use variables just to name an intermediate
73+
value; use the expression directly unless it aids readability
74+
- When adding new e2e tests, run `make update` to regenerate
75+
.openshift-tests-extension metadata — CI verify-update will fail
76+
otherwise
77+
- path: "test/**/*.go"
78+
instructions: |
79+
E2E test conventions:
80+
- Add comments explaining non-obvious test URLs or external endpoints
81+
- Use Ginkgo Labels to mark test categories (e.g., TechPreview, serial)
82+
- When skipping tests for certain environments, document the reason
83+
- Ensure test names follow the
84+
`[Jira:"Cluster Version Operator"] description` format
85+
tools:
86+
shellcheck:
87+
enabled: true
88+
markdownlint:
89+
enabled: true
90+
auto_review:
91+
enabled: true
92+
drafts: true
93+
chat:
94+
auto_reply: true

CONTRIBUTING.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,12 @@ The format can be described more formally as follows:
6262
The first line is the subject and should be no longer than 70 characters, the second line is always blank, and other lines should be wrapped at 80 characters.
6363
This allows the message to be easier to read on GitHub as well as in various Git tools.
6464

65+
## Automated Code Review
66+
67+
Review rules are defined in [`.coderabbit.yaml`](.coderabbit.yaml) and encode
68+
expected patterns for new submissions. Please review these rules when
69+
contributing to understand the standards enforced during automated review.
70+
6571
[golang-style]: https://github.com/golang/go/wiki/CodeReviewComments
6672
[new-issue]: https://github.com/openshift/cluster-version-operator/issues/new
6773
[prow-review]: https://github.com/kubernetes/community/blob/master/contributors/guide/owners.md#the-code-review-process

0 commit comments

Comments
 (0)