Skip to content

Commit 789bcc7

Browse files
dgoodwinclaude
andcommitted
Add CodeRabbit config with review rules derived from PR feedback
Analyzed 100 recent merged PRs and identified recurring review patterns from 14 reviewers. Added path_instructions covering: PrometheusRule alert conventions, manifest naming, state mutation ordering, test structure, container security context, and documentation accuracy. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 02f6e7b commit 789bcc7

1 file changed

Lines changed: 94 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

0 commit comments

Comments
 (0)