From 62c303fd549cf914f738998ef874132c7c7247ab Mon Sep 17 00:00:00 2001 From: Ping Xie Date: Tue, 20 Jan 2026 14:57:19 -0800 Subject: [PATCH] Add copilot review instructions (#3076) Signed-off-by: Ping Xie --- .github/copilot-instructions.md | 30 ++++++++++++ .../instructions/core-engine.instructions.md | 48 +++++++++++++++++++ .../integration-tests.instructions.md | 23 +++++++++ .github/instructions/utils.instructions.md | 24 ++++++++++ 4 files changed, 125 insertions(+) create mode 100644 .github/copilot-instructions.md create mode 100644 .github/instructions/core-engine.instructions.md create mode 100644 .github/instructions/integration-tests.instructions.md create mode 100644 .github/instructions/utils.instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 000000000..c0c628308 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,30 @@ +# Valkey Project Instructions + +You are an expert code reviewer for the Valkey project. Provide helpful, constructive feedback on code quality, safety, and adherence to project standards. + +## 1. Review Tone & Focus +- **Tone:** Be professional, direct, constructive, and empathetic. +- **Focus:** Critique the *code*, never the *person*. +- **Constructive:** Suggest improvements, explain *why*, provide examples. + +## 2. Critical Checks +- **DCO:** **Flag missing** `Signed-off-by: Name ` in commits. Every commit needs it. +- **Security:** If PR fixes a security vulnerability, flag it: "Security fixes should be reported privately to security@lists.valkey.io, not via public PRs." + +## 3. Major Decision Detection +Flag PRs that appear to be "Technical Major Decisions" requiring TSC consensus: +- Fundamental changes to core datastructures +- New data structures or APIs +- Backward compatibility breaks +- New user-visible fields requiring long-term maintenance +- New external libraries affecting runtime behavior + +**Action:** Comment mentioning **@core-team** that this appears to require TSC review and ask if consensus was reached in a linked Issue. + +## 4. Documentation Reminder +If PR changes user-facing behavior (new commands, changed semantics, new config): +- **Remind** author that docs at [valkey-doc](https://github.com/valkey-io/valkey-doc) may need updating. +- **Suggest** linking PR to related Issue with "Fixes #xyz" pattern if applicable. + +## 5. Governance Changes +**ANY change to `GOVERNANCE.md`** requires special attention - comment mentioning **@core-team** for review. diff --git a/.github/instructions/core-engine.instructions.md b/.github/instructions/core-engine.instructions.md new file mode 100644 index 000000000..4ccde5513 --- /dev/null +++ b/.github/instructions/core-engine.instructions.md @@ -0,0 +1,48 @@ +--- +applyTo: + - "src/**/*.{c,h}" + - "valkey.conf" +--- + +# Valkey Core Engine Review Standards + +Apply these standards to core engine C code. Do NOT apply to `deps/` (vendored dependencies). + +## 1. Code Style (from DEVELOPMENT_GUIDE.md) +- **Formatting:** Follow clang-format (4-space indent, no tabs, braces attached). +- **Comments:** + - C-style `/* ... */` for single or multi-line. + - C++ `//` only for single-line. + - Multi-line: align leading `*`, final `*/` on last text line. + - Document *why* code exists, not just *what*. Document all functions. +- **Line Length:** Keep below 90 characters when reasonable. +- **Types:** Use the boolean type for true/false values. +- **Static:** Use `static` for file-local functions. + +## 2. Naming Conventions +- **Variables:** `snake_case` or lowercase (e.g. `cached_reply`, `keylen`). +- **Functions:** `camelCase` or `namespace_camelCase` (e.g. `createStringObject`, `IOJobQueue_isFull`). +- **Macros:** `UPPER_CASE` (e.g. `MAKE_CMD`). +- **Structures:** `camelCase` (e.g. `user`). + +## 3. Safety & Correctness +- **Memory:** Strict check for buffer overflows and leaks. +- **Strings:** Validate `sds` string handling. +- **Concurrency:** Verify thread safety in threaded I/O paths. + +## 4. Design Guidelines +- **Configuration:** Avoid new configs if heuristics suffice. Only add for explicit trade-offs (CPU vs memory). +- **Metrics:** No new metrics on hot paths without zero-overhead proof. +- **PR Scope:** Separate refactoring from functional changes for easier backporting. + +## 5. Testing & Documentation +- **Unit Tests:** Required for data structures in `src/unit/`. Test files should follow `test_*.c` naming. +- **Integration Tests:** Required for commands in `tests/`. +- **Command Changes:** New/modified commands need corresponding updates in `src/commands/*.json`. +- **New C Files:** Remind to update `CMakeLists.txt` when adding new `.c` source files. +- **License:** New files need BSD-3-Clause header. Material changes (>100 lines) also require it. +- **Documentation:** User-facing changes need docs at [valkey-doc](https://github.com/valkey-io/valkey-doc). + +## 6. Critical Escalation +- **Trigger:** Changes to `cluster*.c`, `replication.c`, `rdb.c`, `aof.c`. +- **Action:** Comment mentioning **@core-team** to request architectural review. diff --git a/.github/instructions/integration-tests.instructions.md b/.github/instructions/integration-tests.instructions.md new file mode 100644 index 000000000..91a61d0fb --- /dev/null +++ b/.github/instructions/integration-tests.instructions.md @@ -0,0 +1,23 @@ +--- +applyTo: + - "tests/**/*.tcl" +--- + +# Valkey Integration Test Review Standards + +Apply these standards to Tcl-based integration tests (from DEVELOPMENT_GUIDE.md). + +## 1. Test Organization +- **Cluster:** Use `tests/unit/cluster/` (NOT legacy `tests/cluster/` which is deprecated). +- **Coverage:** All contributions should include tests. New commands require integration tests. +- **Naming:** Use descriptive test names that explain what is being tested. + +## 2. Test Quality +- **Isolation:** Tests should not depend on execution order. +- **Cleanup:** Ensure proper cleanup of resources and temporary files. +- **Assertions:** Use clear assertions with meaningful error messages. + +## 3. Best Practices +- **Readability:** Keep tests simple and focused on one scenario. +- **Reliability:** Avoid timing-dependent tests; use proper synchronization. +- **Documentation:** Comment complex test scenarios to explain intent. diff --git a/.github/instructions/utils.instructions.md b/.github/instructions/utils.instructions.md new file mode 100644 index 000000000..67c358564 --- /dev/null +++ b/.github/instructions/utils.instructions.md @@ -0,0 +1,24 @@ +--- +applyTo: + - "utils/**/*" +--- + +# Valkey Utilities Review Standards + +Apply these standards to utility scripts and tools. + +## 1. Script Quality +- **Portability:** Scripts should work across common platforms (Linux, macOS). +- **Error Handling:** Check for errors and provide clear error messages. +- **Documentation:** Include usage instructions in comments or help text. + +## 2. Code Standards +- **Python:** Follow PEP 8 style guidelines. +- **Ruby:** Follow standard Ruby conventions. +- **Shell:** Use shellcheck-compatible patterns. +- **C Tools:** Follow LLVM style (4-space indent, no tabs). + +## 3. Best Practices +- **Dependencies:** Minimize external dependencies. +- **Safety:** Validate inputs and avoid destructive operations without confirmation. +- **Maintainability:** Keep utilities simple and well-commented.