Conversation
| Before reviewing, read: | ||
|
|
||
| - `AGENTS.md` | ||
| - `docs/ai-review.md` |
There was a problem hiding this comment.
What is the benefit of moving to a separate file?
| 1. Identify the changed module and any compatibility-sensitive surfaces. | ||
| 2. Check correctness, regressions, API and behavior stability, JDBC or R2DBC semantics, and missing focused tests before style suggestions. |
| 1. Identify the changed module and any compatibility-sensitive surfaces. | ||
| 2. Check correctness, regressions, API and behavior stability, JDBC or R2DBC semantics, and missing focused tests before style suggestions. | ||
| 3. Use common Java and driver behavior as a comparison aid, but preserve existing project behavior unless the change is intentionally user-visible. | ||
| 4. Report findings first, ordered by severity, then open questions or assumptions, then a brief summary. |
There was a problem hiding this comment.
ordered by severity
how to define severity?
| Use it as a compact checklist: | ||
|
|
||
| - Confirm the change does not remove or alter any listed feature unintentionally. | ||
| - Add or update tests when a change touches a listed feature in a compatibility-sensitive way. |
There was a problem hiding this comment.
I would also add that end-to-end and integration tests should never be modified if the API surface has not been changed
|
|
||
| ## `client-v2` | ||
|
|
||
| - HTTP and HTTPS connectivity: Connects to ClickHouse over HTTP(S), supports endpoint paths, and exposes a basic `ping` health check. |
There was a problem hiding this comment.
It's a great list. How can we map it to client components?
|
|
||
| In final summaries, state: | ||
|
|
||
| - what changed |
There was a problem hiding this comment.
can we remove duplicates? for example, this one is duplicated in docs/ai-review
|
|
||
| ## Optional nested AGENTS.md | ||
|
|
||
| If a module develops its own conventions, add a nested `AGENTS.md` inside that module. |
There was a problem hiding this comment.
Are you going to add separate AGENTS.md for client and JDBC?
| @@ -0,0 +1,153 @@ | |||
| # Copilot Instructions for `clickhouse-java` | |||
There was a problem hiding this comment.
I'm a bit worried that the content overlaps with other files, can we consolidate agent instructions (not skills!) in agents.md and point .github/copilot-instructions.md to it?
|
|
||
| Before reviewing, read: | ||
|
|
||
| - `AGENTS.md` |
There was a problem hiding this comment.
I don't think is necessary, the harness will put AGENTS.md in the context by default.
|
|
||
| ## Project overview | ||
|
|
||
| `clickhouse-java` is a multi-module Maven repository for ClickHouse Java clients and drivers. |
There was a problem hiding this comment.
Should we also specify the minimum JVM version
There was a problem hiding this comment.
and minimum supported Java version?
|
|
||
| `clickhouse-java` is a multi-module Maven repository for ClickHouse Java clients and drivers. | ||
|
|
||
| Main modules: |
There was a problem hiding this comment.
Should we list the benchmark module?
4 participants
Summary
Checklist
Delete items not relevant to your PR: