NO-ISSUE: Add CodeRabbit config with review rules from PR analysis#1360
NO-ISSUE: Add CodeRabbit config with review rules from PR analysis#1360
Conversation
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>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdded a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
dgoodwin
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dgoodwin 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 |
|
This is just a beta test, was hoping someone might have time to tell me how useful these look. Testing a claude command to try to cut down human review effort by analyzing past reviews in the repo and adding coderabbit rules. Let me know what you think of the results, or close if you don't have time. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.coderabbit.yaml (1)
65-77: Unit-test and e2e path instructions overlap; narrow one scope.
**/*_test.goalso matches files undertest/**, so e2e tests may receive both instruction sets. Consider scoping unit-test guidance to non-e2e directories.Example refinement
- - path: "**/*_test.go" + - path: "pkg/**/*_test.go"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.coderabbit.yaml around lines 65 - 77, The unit-test glob "**/*_test.go" overlaps with the e2e glob "test/**/*.go"; update .coderabbit.yaml to prevent dual matching by narrowing the unit-test rule—either add an exclude for the e2e directory (e.g., keep "**/*_test.go" but add an exclude entry "test/**") or replace the unit-test path with a more specific scope (e.g., "pkg/**/*_test.go" or "internal/**/*_test.go"); ensure the two entries ("**/*_test.go" and "test/**/*.go") no longer both match the same files so e2e tests only get the e2e instructions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.coderabbit.yaml:
- Line 84: Update the Jira test-name format in the rule that currently uses
`[Jira:"Cluster Version Operator"] description` to match the PR objective by
replacing the long name with the acronym, i.e. change instances of `Cluster
Version Operator` to `CVO` so the pattern becomes `[Jira:"CVO"] description`;
locate the entry in .coderabbit.yaml (the rule emitting the `[Jira:"Cluster
Version Operator"]` token) and update the string or regex there so the enforced
convention matches the PR objective.
- Line 15: The glob pattern "install/**prometheusrule*" in .coderabbit.yaml is
malformed because "**" must be followed by "/" to match directories; update the
path to use a proper minimatch recursive segment such as
"install/**/prometheusrule*" or a more specific filename pattern (e.g.,
"install/**/prometheusrule*.yaml") so the rule correctly matches PrometheusRule
files; locate and replace the path value for the rule that currently uses
"install/**prometheusrule*" (search for the exact string
"install/**prometheusrule*") with the corrected glob.
---
Nitpick comments:
In @.coderabbit.yaml:
- Around line 65-77: The unit-test glob "**/*_test.go" overlaps with the e2e
glob "test/**/*.go"; update .coderabbit.yaml to prevent dual matching by
narrowing the unit-test rule—either add an exclude for the e2e directory (e.g.,
keep "**/*_test.go" but add an exclude entry "test/**") or replace the unit-test
path with a more specific scope (e.g., "pkg/**/*_test.go" or
"internal/**/*_test.go"); ensure the two entries ("**/*_test.go" and
"test/**/*.go") no longer both match the same files so e2e tests only get the
e2e instructions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 55fc7780-02f7-4cc7-81d9-416d2be9c3c4
📒 Files selected for processing (1)
.coderabbit.yaml
| review_status: true | ||
| collapse_walkthrough: false | ||
| path_instructions: | ||
| - path: "install/**prometheusrule*" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify candidate PrometheusRule-related files and inspect current glob entry.
# Expected: output should include all intended files under install/, then confirm
# whether current pattern is precise enough or should be changed to install/**/*prometheusrule*.
set -euo pipefail
echo "Current config line:"
rg -n 'path:\s*"install/\*\*prometheusrule\*"' .coderabbit.yaml -n -C2 || true
echo
echo "Candidate files under install containing prometheusrule (case-insensitive):"
fd -i 'prometheusrule' install || true
echo
echo "All install YAML/YML files for manual spot-check:"
fd -e yaml -e yml . install || trueRepository: openshift/cluster-version-operator
Length of output: 1616
🌐 Web query:
CodeRabbit path_instructions glob pattern syntax documentation
💡 Result:
CodeRabbit's path_instructions feature uses glob patterns to apply custom review instructions to specific file paths or directories in the .coderabbit.yaml configuration file. Paths accept glob patterns following the minimatch library syntax. Configuration example: reviews: path_instructions: - path: "src/controllers/" instructions: | - Focus on authentication, authorization, and input validation. - Flag any direct database queries that bypass the ORM layer. - path: "tests/" instructions: | Review the following unit test code written using the Mocha test library. Ensure that: - The code adheres to best practices associated with Mocha. - Descriptive test names are used to clearly convey the intent of each test. - Edge cases and error paths are covered. - path: "docs/.md" instructions: | Check for clarity, accuracy, and completeness. This can also be configured via the Web UI in Organization Settings → Reviews → Behavior. Glob Pattern Syntax (via minimatch): - Use forward slashes (/) as path separators, even on Windows. - * : matches zero or more characters in a path segment. - : matches any number of path segments (directories), including none. - ? : matches one character. - {a,b} : brace expansion, matches 'a' or 'b'. - [abc] : character class, matches a, b, or c. - [!abc] or [a-z] : negated or range. - Patterns starting with ! are for exclusion in path_filters, but path_instructions uses positive matching. - # at start is a comment. For full minimatch documentation, see https://isaacs.github.io/minimatch/. Examples from docs: - "/.ts" for TypeScript files. - "src/api/" for API directory. - "tests/." for all test files. Instructions max length: 20000 characters per entry.
Citations:
- 1: https://docs.coderabbit.ai/configuration/path-instructions
- 2: https://docs.coderabbit.ai/guides/review-instructions?%3Futm_campaign=rubocop
- 3: https://coderabbit.mintlify.app/configuration/path-instructions
- 4: https://isaacs.github.io/minimatch/
- 5: https://docs.coderabbit.ai/changelog/path-based-review-instructions
- 6: https://aicodereview.cc/blog/coderabbit-configuration
🏁 Script executed:
# Search for CodeRabbit schema or documentation in the repo
find . -name "*.md" -o -name "*.txt" -o -name "schema*" | head -20
rg -i "path_instructions\|glob.*pattern" --type md --type txt -A 2 -B 2 || true
rg -i "path:.*install.*\*\*" . -t yaml || trueRepository: openshift/cluster-version-operator
Length of output: 870
PrometheusRule path glob does not follow minimatch syntax properly.
The pattern install/**prometheusrule* is malformed. Per CodeRabbit's minimatch glob syntax, ** must be followed by / when used to match directory levels. Use an explicit recursive segment form to correctly target files.
Proposed fix
- - path: "install/**prometheusrule*"
+ - path: "install/**/*prometheusrule*"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - path: "install/**prometheusrule*" | |
| - path: "install/**/*prometheusrule*" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.coderabbit.yaml at line 15, The glob pattern "install/**prometheusrule*" in
.coderabbit.yaml is malformed because "**" must be followed by "/" to match
directories; update the path to use a proper minimatch recursive segment such as
"install/**/prometheusrule*" or a more specific filename pattern (e.g.,
"install/**/prometheusrule*.yaml") so the rule correctly matches PrometheusRule
files; locate and replace the path value for the rule that currently uses
"install/**prometheusrule*" (search for the exact string
"install/**prometheusrule*") with the corrected glob.
| - Use Ginkgo Labels to mark test categories (e.g., TechPreview, serial) | ||
| - When skipping tests for certain environments, document the reason | ||
| - Ensure test names follow the | ||
| `[Jira:"Cluster Version Operator"] description` format |
There was a problem hiding this comment.
Jira test-name format does not match the PR objective.
This says [Jira:"Cluster Version Operator"] description, but the objective specifies [Jira:"CVO"] description. Please align to avoid enforcing the wrong convention.
Proposed fix
- - Ensure test names follow the
- `[Jira:"Cluster Version Operator"] description` format
+ - Ensure test names follow the
+ `[Jira:"CVO"] description` format🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.coderabbit.yaml at line 84, Update the Jira test-name format in the rule
that currently uses `[Jira:"Cluster Version Operator"] description` to match the
PR objective by replacing the long name with the acronym, i.e. change instances
of `Cluster Version Operator` to `CVO` so the pattern becomes `[Jira:"CVO"]
description`; locate the entry in .coderabbit.yaml (the rule emitting the
`[Jira:"Cluster Version Operator"]` token) and update the string or regex there
so the enforced convention matches the PR objective.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
CONTRIBUTING.md (1)
68-71:⚠️ Potential issue | 🟠 MajorFix incomplete and grammatically incorrect sentences.
The text spanning lines 68-71 contains structural issues:
- Line 68 ends with "encode" but the sentence continues on line 70 with "expected patterns", leaving a duplicate line in between.
- Line 71 is grammatically incomplete, starting with "contributing to understand..." without a clear subject or connection to the previous sentence.
Consider revising the entire paragraph for clarity and correctness. The past review comment from wking suggests cleaner wording that addresses these issues.
📝 Proposed fix based on wking's suggestion
-Review rules are defined in [`.coderabbit.yaml`](.coderabbit.yaml) and encode -Review rules are defined in [`.coderabbit.yaml`](.coderabbit.yaml) and encode -expected patterns for new submissions. -contributing to understand the standards enforced during automated review. +Review rules are defined in [`.coderabbit.yaml`](.coderabbit.yaml) and encode +expected patterns for new submissions.Or, as wking suggested:
-Review rules are defined in [`.coderabbit.yaml`](.coderabbit.yaml) and encode -Review rules are defined in [`.coderabbit.yaml`](.coderabbit.yaml) and encode -expected patterns for new submissions. -contributing to understand the standards enforced during automated review. +Review rules are defined in [`.coderabbit.yaml`](.coderabbit.yaml) and encode +expected patterns for new submissions. Refer to it when contributing to +understand the standards enforced during automated review.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CONTRIBUTING.md` around lines 68 - 71, Remove the duplicated line and rewrite the paragraph so it reads as one clear sentence: state that review rules are defined in `.coderabbit.yaml` and encode expected patterns for new submissions, and then link that to CONTRIBUTING.md describing the standards enforced during automated review; specifically, edit the repeated "Review rules are defined..." lines and the fragment starting "contributing to understand..." into a single grammatical sentence that conveys those two points (use `.coderabbit.yaml` and CONTRIBUTING.md as the referenced artifacts).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CONTRIBUTING.md`:
- Around line 68-69: The CONTRIBUTING.md contains a duplicated sentence "Review
rules are defined in [`.coderabbit.yaml`](.coderabbit.yaml) and encode" on
consecutive lines (lines shown in the diff); remove the redundant second
occurrence so the sentence appears only once and ensure surrounding paragraph
spacing remains correct.
---
Duplicate comments:
In `@CONTRIBUTING.md`:
- Around line 68-71: Remove the duplicated line and rewrite the paragraph so it
reads as one clear sentence: state that review rules are defined in
`.coderabbit.yaml` and encode expected patterns for new submissions, and then
link that to CONTRIBUTING.md describing the standards enforced during automated
review; specifically, edit the repeated "Review rules are defined..." lines and
the fragment starting "contributing to understand..." into a single grammatical
sentence that conveys those two points (use `.coderabbit.yaml` and
CONTRIBUTING.md as the referenced artifacts).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 96836bec-88c7-4f4a-8b56-12d75543a090
📒 Files selected for processing (1)
CONTRIBUTING.md
CONTRIBUTING.md
Outdated
| Review rules are defined in [`.coderabbit.yaml`](.coderabbit.yaml) and encode | ||
| Review rules are defined in [`.coderabbit.yaml`](.coderabbit.yaml) and encode |
There was a problem hiding this comment.
Remove duplicate line.
Lines 68 and 69 are identical. This appears to be a copy-paste error.
🔧 Proposed fix
Review rules are defined in [`.coderabbit.yaml`](.coderabbit.yaml) and encode
-Review rules are defined in [`.coderabbit.yaml`](.coderabbit.yaml) and encode
expected patterns for new submissions.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Review rules are defined in [`.coderabbit.yaml`](.coderabbit.yaml) and encode | |
| Review rules are defined in [`.coderabbit.yaml`](.coderabbit.yaml) and encode | |
| Review rules are defined in [`.coderabbit.yaml`](.coderabbit.yaml) and encode | |
| expected patterns for new submissions. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CONTRIBUTING.md` around lines 68 - 69, The CONTRIBUTING.md contains a
duplicated sentence "Review rules are defined in
[`.coderabbit.yaml`](.coderabbit.yaml) and encode" on consecutive lines (lines
shown in the diff); remove the redundant second occurrence so the sentence
appears only once and ensure surrounding paragraph spacing remains correct.
9136078 to
77a149a
Compare
|
@dgoodwin: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
wking
changed the title
|
/verified by reading the diff, and confirming this doesn't touch any code we ship to customers or test coverage, it's all dev-docs and presubmit tooling |
2 participants
Summary
.coderabbit.yamlwithinheritance: trueto pick up org-wide rules from openshift/coderabbitpath_instructionsrules targeting patterns reviewers consistently catch:last_over_time(), prefer!= 0, singular metric names, verify runbook URLsreadOnlyRootFilesystemon init containers too, justify exceptionsmake update[Jira:"CVO"] descriptionnamingTest plan
.coderabbit.yamlparses correctly (CodeRabbit will validate on this PR)Patterns derived from review comments by: wking, hongkailiu, DavidHurta, petr-muller, simonpasquier, JoelSpeed, JianLi-RH, ehearne-redhat, dusk125, lance5890, deads2k, tchap, jiajliu, dis016
🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Documentation