-
Notifications
You must be signed in to change notification settings - Fork 61
agents: update agents.md #5017
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
agents: update agents.md #5017
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| # Error Handling Guidelines | ||
|
|
||
| Read this before adding or changing error creation, wrapping, or propagation. | ||
|
|
||
| - Use predefined errors from the repository error package; keep the local import name consistent with surrounding code. | ||
| - When an error comes from a third-party or library call, wrap it immediately at the boundary with `errors.WrapError(predefinedError, err, args...)`. | ||
| - For new or changed code, do not use `errors.Trace` as the initial wrapper for third-party or library errors. If no predefined error fits, choose an existing predefined error or add an appropriate one, then use `errors.WrapError`. | ||
| - After an error has been wrapped with `errors.WrapError`, propagate it directly; do not call `errors.Trace` on later paths. | ||
| - When creating a TiCDC error, use `GenWithStack...` or `GenWithStackByArgs...` on a predefined error and pass concrete details through arguments when needed. | ||
| - Decide whether a newly generated error needs stack information. If a stack is unnecessary, especially on hot paths, use `FastGen...` or `FastGenByArgs...`. | ||
| - Avoid other error creation or wrapping styles in new or changed code, including `errors.New`, bare `fmt.Errorf`, and adding stack information multiple times. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| # Generated Code Guidelines | ||
|
|
||
| Read this before changing protobufs, mocks, generated Go files, dashboards, or files produced by `go generate`. | ||
|
|
||
| ## Principles | ||
|
|
||
| - Prefer editing the source definition and regenerating output over manually patching generated files. | ||
| - Keep generated diffs paired with the source change that requires them. | ||
| - If generation produces unrelated churn, stop and inspect before including it. | ||
| - Do not commit local tool binaries, build outputs, coverage files, or temporary artifacts. | ||
|
|
||
| ## Generation Commands | ||
|
|
||
| - Protobufs: run `make generate-protobuf` after changing `eventpb/**/*.proto`, `heartbeatpb/**/*.proto`, or `logservice/logservicepb/**/*.proto`. | ||
| - Mocks: run `make generate_mock` after changing interfaces listed in `scripts/generate-mock.sh` or adding a mock to that flow. | ||
| - Go generate: run `make go-generate` after changing files whose generated output is controlled by `//go:generate`. | ||
| - Next generation Grafana dashboards: run `make generate-next-gen-grafana` after changing inputs consumed by `scripts/generate-next-gen-metrics.sh`. | ||
| - Full pre-submit generation check: run `make check` when a change may affect formatting, generated files, dashboards, Makefile formatting, or module tidiness. | ||
|
|
||
| ## Review Checklist | ||
|
|
||
| - Confirm generated files are deterministic and limited to the intended source change. | ||
| - Confirm generated files are not manually edited without a source-of-truth update. | ||
| - Confirm newly required generated files are included in the diff. | ||
| - Confirm no tool downloads, binaries, or temporary files are included. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| # Logging Guidelines | ||
|
|
||
| Read this before adding, removing, or rewriting logs. | ||
|
|
||
| - Use structured logs via `github.com/pingcap/log` with `zap` fields. | ||
| - Treat logs as operational signals, not control-flow comments. Keep normal paths quiet. | ||
| - Default `INFO`/`WARN` logs should record high-value lifecycle events, state changes, external dependency abnormalities, or invariant violations. | ||
| - Choose log levels by required action: | ||
| - `ERROR`: correctness, availability, or key progress is affected and needs attention. | ||
| - `WARN`: the system is abnormal but can continue through recovery, retry, fallback, or degraded behavior. | ||
| - `INFO`: key lifecycle events, important state changes, important configuration, or summary information. | ||
| - `DEBUG`: bounded, low-frequency diagnostics with clear troubleshooting value. | ||
| - Do not add `DEBUG` logs by default. Delete low-value logs instead of moving them to `DEBUG`. | ||
| - Keep `message` stable and concise: summarize what happened, why it happened, and what the system will do next. | ||
| - Put object details in stable camelCase `zap` fields such as `changefeedID`, `nodeID`, `dispatcherID`, `regionID`, `subscriptionID`, and `requestID`. | ||
| - Message strings should not include function names and should avoid `-` (use spaces instead). | ||
| - Avoid per-object or per-iteration logs, duplicated logs on the same error path, large objects, raw payloads, and long error dumps in default logs. | ||
| - Use metrics for counts, scale, frequency, and trends; use windowed summaries or representative samples for high-cardinality events. | ||
| - Before adding, keeping, or rewriting a log, verify that it answers a real diagnostic question, identifies the object, reason, action, and impact, and will not grow linearly with object count or retry/loop frequency. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| # Repository Map | ||
|
|
||
| Read this when choosing where to make a change or which tests to run. | ||
|
|
||
| ## Runtime Components | ||
|
|
||
| - `cmd/`: buildable binaries such as `cmd/cdc`, `cmd/kafka-consumer`, `cmd/storage-consumer`, and helper tools. | ||
| - `server/`: server bootstrap and runtime service wiring. | ||
| - `coordinator/`: changefeed metadata, scheduling coordination, operators, drain, and GC coordination. | ||
| - `maintainer/`: table/span replication ownership, scheduling, split/range checks, and replica lifecycle. | ||
| - `logservice/`: eventstore, logpuller, schema store, transaction utilities, and protobuf definitions for log service internals. | ||
| - `downstreamadapter/`: dispatcher orchestration, routing, event collection, sinks, and syncpoint handling. | ||
|
|
||
| ## Shared Libraries | ||
|
|
||
| - `pkg/config`: configuration types and validation. | ||
| - `pkg/errors`: predefined TiCDC errors and error helpers. | ||
| - `pkg/sink`: shared sink implementations, codecs, and sink utilities. | ||
| - `pkg/filter`, `pkg/binlog-filter`, and `pkg/integrity`: filtering and integrity-related logic. | ||
| - `pkg/etcd`, `pkg/pdutil`, `pkg/security`, and `pkg/server`: external dependency clients and shared service utilities. | ||
| - `pkg/orchestrator`, `pkg/scheduler`, and `pkg/messaging`: shared control-plane and messaging primitives. | ||
|
|
||
| ## Tests and Tooling | ||
|
|
||
| - `tests/integration_tests/`: script-driven integration suites for MySQL, Kafka, storage, and Pulsar. | ||
| - `scripts/`: generation, lint, formatting, and integration helper scripts. | ||
| - `tools/`: pinned local tooling used by Make targets. | ||
| - `metrics/`: Grafana and next-generation dashboard assets. | ||
|
|
||
| ## Placement Rules | ||
|
|
||
| - Put component-owned behavior close to the owning component instead of adding cross-cutting helpers prematurely. | ||
| - Put shared code under `pkg/` only when at least two components need the same abstraction. | ||
| - Prefer existing package tests before adding a new test package or broader integration test. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| # Testing Guidelines | ||
|
|
||
| Read this before adding or changing tests. | ||
|
|
||
| - Unit tests: `*_test.go`, favor deterministic tests; use `testify/require`. | ||
| - Unit tests should cover meaningful behavior only; avoid redundant or low-value cases. | ||
| - Do not test across feature boundaries. Keep each test focused on the behavior owned by the package or component under test. | ||
| - Reuse existing tests when possible. Add a new test only when reuse would make the existing test unclear or incomplete. | ||
| - If several test functions are highly related, merge them into one concise table-driven or scenario-based test. | ||
| - When a test needs mocked components, prefer existing gomock-generated mocks over handwritten mocks. If the required mock does not exist, add it to the mock generation flow and run `make generate_mock`. | ||
| - Keep tests efficient, simple, focused, and easy to update. | ||
| - Failpoints: `make unit_test` enables/disables automatically. If you enable manually, disable before committing to avoid a dirty tree. | ||
| - For documentation-only changes, unit tests are usually unnecessary. If tests are skipped, state in the final response that only documentation was changed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| # Validation Guidelines | ||
|
|
||
| Read this before choosing build, test, lint, or integration checks. | ||
|
|
||
| ## Principles | ||
|
|
||
| - Start with the narrowest command that covers the changed behavior, then broaden only when risk or confidence requires it. | ||
| - Prefer package-scoped unit tests for code changes and reserve full-suite or integration tests for cross-component, protocol, sink, or deployment behavior. | ||
| - If a command is too expensive or requires unavailable services, state what was not run and why in the final response. | ||
| - Do not fix unrelated failures. Capture the failing command and the first relevant failure, then report it separately. | ||
|
|
||
| ## Common Commands | ||
|
|
||
| - Build the main binary with `make cdc`. | ||
| - Format Go, shell, imports, and log style with `make fmt`. | ||
| - Run pre-submit checks with `make check`. | ||
| - Run all unit tests with `make unit_test`. | ||
| - Run focused unit tests with `make unit_test_pkg PKG=./pkg/sink/...`. | ||
| - Run targeted integration suites with `make integration_test_mysql CASE=<name>`, `make integration_test_kafka CASE=<name>`, `make integration_test_storage CASE=<name>`, or `make integration_test_pulsar CASE=<name>`. | ||
|
|
||
| ## Task-to-Validation Matrix | ||
|
|
||
| - `pkg/` library changes: run `make unit_test_pkg PKG=./pkg/<package>/...`; broaden to `make unit_test` for shared utilities used widely. | ||
| - `downstreamadapter/sink/` changes: run the package unit tests, then the matching sink integration suite when behavior crosses process or external-system boundaries. | ||
| - `coordinator/`, `maintainer/`, or scheduling changes: run focused package tests and consider `make unit_test` when ownership, lifecycle, or concurrency invariants change. | ||
| - `logservice/` changes: run focused logservice package tests; consider broader unit tests for eventstore, logpuller, schema, or txn boundary changes. | ||
| - API, config, or CLI changes: run focused package tests and `make cdc` when command wiring or config loading changes. | ||
| - Documentation-only changes: unit tests are usually unnecessary; run markdown or whitespace checks when practical and state that only documentation changed. | ||
|
|
||
| ## Reporting | ||
|
|
||
| In the final response, include the commands run, their result, and any checks skipped with a short reason. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.