Skip to content

Add CodeRabbit review rules from PR review patterns - #8096

Closed
dgoodwin wants to merge 1 commit into
openshift:mainfrom
dgoodwin:coderabbit-rules-from-reviews
Closed

Add CodeRabbit review rules from PR review patterns#8096
dgoodwin wants to merge 1 commit into
openshift:mainfrom
dgoodwin:coderabbit-rules-from-reviews

Conversation

@dgoodwin

@dgoodwin dgoodwin commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR adds CodeRabbit review rules to .coderabbit.yaml based on patterns
identified from analyzing recent merged PR review comments. These rules encode
recurring feedback from human reviewers so that CodeRabbit can catch the same
issues automatically in future PRs.

How these were identified

Analyzed the most recent 100 merged PRs and collected 480 human review comments
from 24 unique reviewers (excluding bots, approvals, and prow commands). Patterns
appearing in 3+ PRs or from 2+ different reviewers were proposed as rules.

Top reviewers represented: JoelSpeed (117), jparrill (113), enxebre (65),
bryan-cox (51), sdminonne (31), cblecker (23).

Re-running

This command can be re-run periodically to identify new patterns:

/teams:coderabbit-rules-from-pr-reviews openshift/hypershift

It compares against existing rules and only proposes net-new additions.

Generated by teams:coderabbit-rules-from-pr-reviews

Summary by CodeRabbit

  • Chores
    • Enhanced development and code review standards to improve consistency and code quality across the project. New guidelines establish specifications for mandatory documentation practices, API validation methodologies, consistent naming conventions, test coverage requirements, and development best practices to ensure elevated quality and maintainability standards.

@openshift-ci-robot

Copy link
Copy Markdown

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@openshift-ci openshift-ci Bot added do-not-merge/needs-area needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 27, 2026
@openshift-ci

openshift-ci Bot commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

Please specify an area label

Details

Instructions 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.

@openshift-ci

openshift-ci Bot commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dgoodwin
Once this PR has been reviewed and has the lgtm label, please assign jparrill for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci
openshift-ci Bot requested review from bryan-cox and devguyio March 27, 2026 17:48
@dgoodwin

Copy link
Copy Markdown
Contributor Author

Please feel free to tell me this is slop and not useful. This is the third repo I'm trying, first two the teams felt this was useful.

Analyzed recent merged PRs to identify recurring review feedback patterns
and codified them as CodeRabbit path_instructions rules. These rules help
catch common issues automatically during code review.

Generated by teams:coderabbit-rules-from-pr-reviews
@coderabbitai

coderabbitai Bot commented Mar 27, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This change adds a new reviews.path_instructions section to .coderabbit.yaml that defines path-scoped rules for code review. It specifies style, validation, and testing guidelines for Go API type files, test files, and general Go files. The rules cover kubebuilder validation markers, enum casing, godoc requirements, OpenShift conventions, string validation markers, discriminated-union validation, CRD API type restrictions, test naming conventions, and code coverage expectations.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@dgoodwin
dgoodwin force-pushed the coderabbit-rules-from-reviews branch from 0550074 to 1bed68d Compare March 27, 2026 17:54
@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 27, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
.coderabbit.yaml (1)

140-146: Scope the test-expectation text to non-test source changes.

Because this instruction lives under **/*.go, it also applies to *_test.go files; tightening wording avoids redundant “add tests” feedback on test-only PRs.

♻️ Suggested wording tweak
-        New exported functions and non-trivial logic changes should include unit
-        tests. Tests should cover:
+        For non-test Go source changes, new exported functions and non-trivial
+        logic changes should include unit tests. Tests should cover:
         1. Happy path
         2. Error/edge cases (malformed input, empty values, nil)
         3. Platform-specific behavior for all relevant platforms (e.g., if adding
            AWS support, consider whether GCP/Azure also need test coverage)
-        Check that corresponding _test.go files are included in the PR.
+        Check that corresponding _test.go files are included in the PR when the
+        change is outside test files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.coderabbit.yaml around lines 140 - 146, The test-expectation text under the
rule that currently starts "New exported functions and non-trivial logic changes
should include unit tests..." is too broad and triggers on *_test.go files;
update that rule so it only applies to non-test Go sources by excluding test
files (i.e., adjust the glob or add an exclusion so the rule no longer matches
*_test.go). Edit the rule text associated with the "**/*.go" scope (the
paragraph beginning "New exported functions...") to both tighten the wording and
ensure the matcher excludes test-only files so PRs that only change *_test.go do
not get the "add tests" reminder.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.coderabbit.yaml:
- Around line 140-146: The test-expectation text under the rule that currently
starts "New exported functions and non-trivial logic changes should include unit
tests..." is too broad and triggers on *_test.go files; update that rule so it
only applies to non-test Go sources by excluding test files (i.e., adjust the
glob or add an exclusion so the rule no longer matches *_test.go). Edit the rule
text associated with the "**/*.go" scope (the paragraph beginning "New exported
functions...") to both tighten the wording and ensure the matcher excludes
test-only files so PRs that only change *_test.go do not get the "add tests"
reminder.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 67c725d4-e6d0-407b-8a2d-c2f23389622c

📥 Commits

Reviewing files that changed from the base of the PR and between c25481f and 1bed68d.

📒 Files selected for processing (1)
  • .coderabbit.yaml

@codecov

codecov Bot commented Mar 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 26.56%. Comparing base (c25481f) to head (1bed68d).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8096   +/-   ##
=======================================
  Coverage   26.56%   26.56%           
=======================================
  Files        1087     1087           
  Lines      105042   105042           
=======================================
  Hits        27902    27902           
  Misses      74731    74731           
  Partials     2409     2409           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@openshift-ci

openshift-ci Bot commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

@dgoodwin: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions 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.

@dgoodwin

Copy link
Copy Markdown
Contributor Author

/close

@openshift-ci openshift-ci Bot closed this Mar 30, 2026
@openshift-ci

openshift-ci Bot commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

@dgoodwin: Closed this PR.

Details

In response to this:

/close

Instructions 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.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants