Skip to content

Commit 8cf3a3f

Browse files
authored
Add CLAUDE.md and CodeRabbit configuration (#3228)
## Short description Add CodeRabbit AI review automation and Claude Code project guidelines to improve code review quality and establish consistent coding standards. ## More details This PR introduces two key components: ### 1. CodeRabbit Configuration (`.coderabbit.yaml`) - Configures automated PR reviews with assertive profile for strict enforcement - Enables multiple linting tools: ruff, mypy, yamllint, shellcheck, gitleaks, semgrep, actionlint, hadolint, markdownlint - Adds path-specific review instructions for `tests/`, `utilities/`, `libs/`, and `conftest.py` - Integrates with project's knowledge base (CLAUDE.md, coding guides) ### 2. Claude Code Guidelines (`CLAUDE.md`) - Documents strict rules: no linter suppressions, code reuse requirements - Establishes Python requirements: type hints, docstrings, uv-only execution - Defines test requirements: single-action fixtures, noun naming, markers - Provides essential commands for running tests and development - Documents code architecture and directory structure - Explains fixture guidelines and PR process ### 3. Gitignore Updates - Added `.claude/` directory (Claude Code local config) - Added `ocp_resources` (generated resources) ## Jira ticket N/A - Infrastructure improvement ## Upstream PR (when applicable) N/A ## Additional info - This enables automated code review feedback on all PRs - Establishes consistent coding standards documentation - Improves onboarding for new contributors <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Enabled automated AI-assisted code review with pre-merge enforcement, incremental auto-reviews, branch-targeting, rich review summaries and display options, chat auto-reply, path/title filtering, expanded ignore rules for local/assistant artifacts, and a broad suite of linting/security checks. * **Documentation** * Added a comprehensive developer guide consolidating coding standards, docstring/testing requirements, workflows, assistant integration, and review/merge procedures. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent e5fba5f commit 8cf3a3f

File tree

3 files changed

+264
-0
lines changed

3 files changed

+264
-0
lines changed

.coderabbit.yaml

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
2+
# CodeRabbit AI Review Configuration for openshift-virtualization-tests
3+
# Based on project-specific rules from CLAUDE.md
4+
5+
language: en-US
6+
7+
tone_instructions: "Be direct and specific. Explain WHY rules exist. Provide code examples. Distinguish critical violations from suggestions. Use severity levels: CRITICAL (blocking/security), HIGH (types/fixtures), MEDIUM (style), LOW (suggestions)."
8+
9+
early_access: true
10+
enable_free_tier: true
11+
12+
reviews:
13+
# Assertive profile for strict enforcement of coding standards
14+
profile: assertive
15+
16+
# Request changes for critical violations
17+
request_changes_workflow: true
18+
19+
# Review display settings
20+
high_level_summary: true
21+
high_level_summary_placeholder: "@coderabbitai summary"
22+
auto_title_placeholder: "@coderabbitai"
23+
review_status: true
24+
commit_status: true
25+
poem: false
26+
collapse_walkthrough: true
27+
sequence_diagrams: false
28+
changed_files_summary: true
29+
30+
# Targeted pre-merge checks - only run on changed files
31+
path_filters:
32+
- "!**/*.md" # Skip markdown-only changes for full review
33+
34+
# Abort review if PR is closed
35+
abort_on_close: true
36+
37+
# Auto-review configuration
38+
auto_review:
39+
enabled: true
40+
auto_incremental_review: true
41+
drafts: false
42+
ignore_title_keywords:
43+
- "WIP"
44+
base_branches:
45+
- main
46+
- cnv-4.20
47+
- cnv-4.19
48+
- cnv-4.18
49+
50+
# Enabled linting and security tools
51+
tools:
52+
# Python linting
53+
ruff:
54+
enabled: true
55+
# Additional Python linting (stricter checks)
56+
pylint:
57+
enabled: true
58+
# Note: mypy is not a supported CodeRabbit tool - type checking is enforced via pre-commit and tox
59+
# YAML validation
60+
yamllint:
61+
enabled: true
62+
# Shell script checking
63+
shellcheck:
64+
enabled: true
65+
# Security scanning
66+
gitleaks:
67+
enabled: true
68+
semgrep:
69+
enabled: true
70+
# GitHub Actions workflow validation
71+
actionlint:
72+
enabled: true
73+
# Dockerfile linting
74+
hadolint:
75+
enabled: true
76+
# Markdown linting
77+
markdownlint:
78+
enabled: true
79+
# GitHub checks integration
80+
github-checks:
81+
enabled: true
82+
timeout_ms: 90000
83+
84+
chat:
85+
auto_reply: true
86+
87+
knowledge_base:
88+
opt_out: false
89+
90+
# Enable code guidelines enforcement from CLAUDE.md and docs
91+
code_guidelines:
92+
enabled: true
93+
filePatterns:
94+
- "CLAUDE.md"
95+
96+
# Enable learning from repository patterns
97+
learnings:
98+
scope: auto
99+
100+
# Enable learning from issues
101+
issues:
102+
scope: auto
103+
104+
# Enable learning from pull requests
105+
pull_requests:
106+
scope: auto

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,5 +125,8 @@ docs/source/rst
125125
# IntelliJ project folder
126126
.idea/
127127

128+
# Claude Code assistant
129+
.claude/
130+
128131
# For local run (debug).
129132
ocp_resources

CLAUDE.md

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
# CLAUDE.md
2+
3+
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
4+
5+
6+
## Strict Rules (MANDATORY)
7+
8+
Based on [myk-org/github-metrics CLAUDE.md](https://github.com/myk-org/github-metrics/blob/main/CLAUDE.md#strict-rules-mandatory)
9+
10+
### Linter Suppressions PROHIBITED
11+
12+
-**NEVER** add `# noqa`, `# type: ignore`, `# pylint: disable`
13+
-**NEVER** disable linter/mypy rules to work around issues
14+
-**FIX THE CODE** - If linter complains, the code is wrong
15+
- If you think a rule is wrong: **ASK** the user for explicit approval
16+
17+
### Code Reuse (Search-First Development)
18+
19+
Before writing ANY new code:
20+
21+
1. **SEARCH** codebase for existing implementations
22+
2. **CHECK** `utilities/` for shared functions
23+
3. **CHECK** `libs/` for shared libraries
24+
4. **CHECK** `tests/` for shared fixtures and helper functions
25+
5. **VERIFY** no similar logic exists elsewhere
26+
6. **NEVER** duplicate logic - extract to shared module
27+
7. **REUSE** existing code and patterns — only write new when nothing exists
28+
29+
### Python Requirements
30+
31+
- **Type hints MANDATORY** - mypy strict mode in `libs/`, all new public functions under utilities MUST be typed
32+
- **Google-format docstrings REQUIRED** - for all public functions with non-obvious return values OR side effects
33+
- **No defensive programming** - fail-fast, don't hide bugs with fake defaults (see exceptions below)
34+
- **ALWAYS use `uv run`** - NEVER execute `python`, `pip`, or `pytest` directly. Use `uv run python`, `uv run pytest`, `uv add` for package installation.
35+
- **ALWAYS use absolute imports** - NEVER use relative imports
36+
- **ALWAYS import specific functions** - use `from module import func`, NEVER `import module`
37+
- **ALWAYS use named arguments** - for function calls with more than one argument
38+
- **NEVER use single-letter variable names** - ALWAYS use descriptive, meaningful names
39+
- **No dead code** - every function, variable, fixture MUST be used or removed. Code marked with `# skip-unused-code` is excluded from dead code analysis (enforced via custom ruff plugin).
40+
- **Prefer direct attribute access** - use `foo.attr` directly. Save to variables only when: reusing the same attribute multiple times improves readability, or extracting clarifies intent.
41+
42+
### Acceptable Defensive Checks (Exceptions Only)
43+
44+
The "no defensive programming" rule has these five exceptions:
45+
46+
1. **Destructors/Cleanup** - May be called during incomplete initialization
47+
2. **Optional Parameters** - Explicitly typed as `Type | None` with default `None`
48+
3. **Lazy Initialization** - Attributes intentionally starting as `None` before first use
49+
4. **Platform/Architecture Constants** - Features unavailable on all platforms (x86_64, arm64, s390x)
50+
5. **Unversioned External Libraries** - External dependencies with unknown API stability
51+
52+
**Still Prohibited (with examples):**
53+
54+
-**Checking attributes that are ALWAYS provided** - Do NOT check if `vm.name` exists when VirtualMachine always has a name field. If the schema guarantees it, trust it.
55+
-**Defensive checks on data guaranteed by architecture** - Do NOT validate that `namespace.client` is not None when the Namespace class always sets client in `__init__`. If the constructor guarantees it, trust it.
56+
-**Using `hasattr()` for type discrimination** - Do NOT use `if hasattr(obj, 'some_method')` to detect type. Use `isinstance(obj, ExpectedType)` for explicit type checking.
57+
-**Version checking for pinned dependencies** - Do NOT check `if kubernetes_version >= X` when pyproject.toml pins the exact version. The lock file guarantees the version.
58+
59+
### Test Requirements
60+
61+
- **All new tests MUST have markers** - check pytest.ini for available markers, NEVER commit unmarked tests
62+
- **Each test verifies ONE aspect only** - single purpose, easy to understand
63+
- **Tests MUST be independent** - use `pytest-dependency` ONLY when test B requires side effects from test A (e.g., cluster-wide configuration).
64+
For resource dependencies, use shared fixtures instead. **When using `@pytest.mark.dependency`, a comment explaining WHY the dependency exists is REQUIRED.**
65+
- **ALWAYS use `@pytest.mark.usefixtures`** - REQUIRED when fixture return value is not used by test
66+
67+
### Fixture Guidelines (CRITICAL)
68+
69+
1. **Single Action REQUIRED**: Fixtures MUST do ONE action only (single responsibility)
70+
2. **Naming REQUIRED**: ALWAYS use NOUNS (what they provide), NEVER verbs
71+
-`vm_with_disk`
72+
-`create_vm_with_disk`
73+
3. **Parametrization format**: Use `request.param` with dict structure for complex parameters
74+
4. **Ordering REQUIRED**: pytest native fixtures first, then session-scoped, then module/class/function scoped
75+
5. **Fixture scope rules**:
76+
- Use `scope="function"` (default) - for setup requiring test isolation
77+
- Use `scope="class"` - for setup shared across test class
78+
- Use `scope="module"` - for expensive setup in a test module
79+
- Use `scope="session"` - for setup that persists the entire test run (e.g., storage class, namespace)
80+
- **NEVER use broader scope if fixture modifies state or creates per-test resources**
81+
82+
83+
### Logging Guidelines
84+
85+
- **INFO level REQUIRED for** - test phase transitions, resource creation/deletion, configuration changes, API responses, intermediate state
86+
- **ERROR level REQUIRED for** - exceptions with full context: what failed, expected vs actual values, resource state
87+
- **NEVER use DEBUG level** - if a log is needed, use INFO.
88+
- **NEVER log** - secrets, tokens, passwords, or PII
89+
- **Log format REQUIRED** - structured key-value pairs for searchability: `LOGGER.info("VM created", extra={"name": vm_name, "namespace": ns})`
90+
91+
### Code Patterns (Not Enforced by Linters)
92+
93+
**Exception Handling:**
94+
- **ALWAYS re-raise with context** - use `raise NewError("message") from original_error` to preserve stack trace
95+
- **Do not catch bare `Exception`** - catch specific exception types only
96+
- **NEVER silently swallow exceptions** - at minimum, log the error before continuing
97+
98+
**Context Managers:**
99+
- **ALWAYS use `with` for resources** - files, connections, locks MUST use context managers
100+
- **Fixtures with cleanup MUST use yield** - use `yield resource` followed by cleanup code, NEVER return + finalizer
101+
102+
**Timeouts and Polling:**
103+
- **ALWAYS use `timeout_sampler`** - from `timeout_sampler` package for any operation that waits for a condition:
104+
```python
105+
from timeout_sampler import TimeoutSampler
106+
for sample in TimeoutSampler(wait_timeout=60, sleep=5, func=check_condition):
107+
if sample:
108+
break
109+
```
110+
- **NEVER use `time.sleep()` in loops** - use `timeout_sampler` with appropriate wait time
111+
112+
**Assertions:**
113+
- **Use pytest assertions** - `assert actual == expected`, NEVER `self.assertEqual()`
114+
- **Include failure messages** - `assert condition, "descriptive message explaining failure"`
115+
116+
**Boolean Checks:**
117+
- **Use implicit boolean** - `if items:` NOT `if len(items) > 0:` or `if items != []:`
118+
- **Use identity for None** - `if x is None:` NOT `if x == None:`
119+
- **NEVER compare to True/False** - `if flag:` NOT `if flag == True:`
120+
121+
### Tests Directory Organization
122+
123+
- **Feature subdirectories REQUIRED** - each feature MUST have its own subdirectory under component (e.g., `tests/network/ipv6/`)
124+
- **Test file naming REQUIRED** - ALWAYS use `test_<functionality>.py` format
125+
- **Local helpers location** - place helper utils in `<feature_dir>/utils.py`
126+
- **Local fixtures location** - place in `<feature_dir>/conftest.py`
127+
- **Move to shared location** - move to `utilities/` or `tests/conftest.py` ONLY when used by different team directories
128+
129+
### Internal API Stability
130+
131+
This is a test suite - internal APIs have NO backward compatibility requirements:
132+
133+
- Return types and method signatures can change freely
134+
- Internal utility functions can be refactored without deprecation
135+
- Only external interfaces (pytest markers, CLI options) need stability
136+
137+
## Essential Commands
138+
139+
### Before commiting Verification (MANDATORY)
140+
141+
Before committing, these checks MUST pass:
142+
143+
```bash
144+
# Required before every commit
145+
pre-commit run --all-files # Linting and formatting
146+
147+
# Full CI checks
148+
tox
149+
150+
# Run utilities unit tests
151+
tox -e utilities-unittests
152+
153+
```
154+
155+
**No exceptions.** Fix all failures before committing. Do not use `--no-verify` to bypass hooks.

0 commit comments

Comments
 (0)