Conversation
📝 WalkthroughWalkthroughA new documentation file Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Around line 14-59: The README/CLAUDE.md lists six Makefile targets that don't
exist: build-tenant-cli, create-empty-secrets, generate-signing-keys,
go-imports-changed, provision-tenants-locally, and start-cmk; update the doc to
match the Makefile by either removing these entries or adding corresponding
rules to the Makefile (implement targets with appropriate recipes and comments),
and ensure targets like go-imports and codegen (api=cmk) remain accurate; verify
by running make <target> for each listed target and updating CLAUDE.md to
reflect the final set of supported targets.
| ```bash | ||
| # Run unit tests (uses gotestsum, clears cache, produces coverage) | ||
| make test | ||
|
|
||
| # Run integration tests (spins up containers for PostgreSQL, Redis, RabbitMQ) | ||
| make integration_test | ||
|
|
||
| # Run linter (golangci-lint v2 with --fix) | ||
| make lint | ||
|
|
||
| # Format all Go files (gofmt + goimports + golines + gci) | ||
| make go-imports | ||
|
|
||
| # Format only changed files | ||
| make go-imports-changed | ||
|
|
||
| # Generate OpenAPI server code from spec | ||
| make codegen api=cmk | ||
|
|
||
| # Run the API server locally | ||
| make run | ||
|
|
||
| # Run benchmarks | ||
| make benchmark | ||
|
|
||
| # Full local K3d cluster setup (PostgreSQL, Redis, RabbitMQ, OTEL, CMK) | ||
| make start-cmk | ||
|
|
||
| # Build dev Docker image | ||
| make docker-dev-build | ||
|
|
||
| # Generate signing keys for local testing | ||
| make generate-signing-keys | ||
|
|
||
| # Create empty secret files from blueprints | ||
| make create-empty-secrets | ||
|
|
||
| # Build tenant-manager CLI locally | ||
| make build-tenant-cli | ||
|
|
||
| # Provision test tenants locally | ||
| make provision-tenants-locally | ||
|
|
||
| # Tidy Go modules | ||
| make tidy | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find CLAUDE.md and Makefile(s)
CLAUDE_FILE="$(fd -HI '^CLAUDE\.md$' -t f | head -n1)"
if [[ -z "${CLAUDE_FILE:-}" ]]; then
echo "CLAUDE.md not found"; exit 1
fi
echo "Using CLAUDE file: $CLAUDE_FILE"
echo
# Extract make targets referenced in the command section
echo "Documented make targets:"
TARGETS=$(awk '
/## Build & Development Commands/{inblk=1}
inblk && /^```bash/{code=1; next}
code && /^```/{exit}
code && $1=="make"{print $2}
' "$CLAUDE_FILE" | sort -u)
printf '%s\n' "$TARGETS"
echo
# Collect declared targets from all Makefiles
echo "Discovered Makefile targets:"
MAKEFILES="$(fd -HI '(^|/)Makefile$' -t f)"
if [[ -z "${MAKEFILES:-}" ]]; then
echo "No Makefile found"; exit 1
fi
DECLARED=$(for mf in $MAKEFILES; do
awk -F: '
/^[A-Za-z0-9_.-]+:[^=]/ {
if ($1 !~ /^\./) print $1
}
' "$mf"
done | sort -u)
printf '%s\n' "$DECLARED"
echo
echo "Missing documented targets:"
comm -23 <(printf '%s\n' "$TARGETS" | sort -u) <(printf '%s\n' "$DECLARED" | sort -u) || trueRepository: openkcm/cmk
Length of output: 694
🏁 Script executed:
fd -HI '^Makefile$' -t fRepository: openkcm/cmk
Length of output: 63
🏁 Script executed:
cat -n Makefile | head -100Repository: openkcm/cmk
Length of output: 4053
Remove or implement the 6 undocumented make targets before merging.
This documentation block references targets that do not exist in the Makefile: build-tenant-cli, create-empty-secrets, generate-signing-keys, go-imports-changed, provision-tenants-locally, and start-cmk. Remove these entries or add the targets to the Makefile. Mismatches will break onboarding and CI debugging workflows.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` around lines 14 - 59, The README/CLAUDE.md lists six Makefile
targets that don't exist: build-tenant-cli, create-empty-secrets,
generate-signing-keys, go-imports-changed, provision-tenants-locally, and
start-cmk; update the doc to match the Makefile by either removing these entries
or adding corresponding rules to the Makefile (implement targets with
appropriate recipes and comments), and ensure targets like go-imports and
codegen (api=cmk) remain accurate; verify by running make <target> for each
listed target and updating CLAUDE.md to reflect the final set of supported
targets.
|
|
||
| - **Commits**: Conventional Commits (`feat:`, `fix:`, `docs:`, `chore:`, `ci:`, `build:`). Release-please automates versioning. | ||
| - **Import ordering** (gci): standard → default → `prefix(github.com/openkcm/cmk)` → blank → dot → alias → localmodule | ||
| - **Linting**: golangci-lint v2 with `default: all` minus disabled linters. JSON tags use `goCamel` case. Exhaustive switch requires `default` to count as exhaustive. |
There was a problem hiding this comment.
Config in .golangcilint
|
|
||
| Errors are mapped per operation in `internal/apierrors/`. Each `ErrorMap` pairs internal error(s) with an HTTP status and error code. The system matches by most specific error chain. To add a new error: | ||
| 1. Define error constant in `apierrors` | ||
| 2. Add `ErrorMap` entry with matching error(s) and `DetailedError` (code, message, status) |
There was a problem hiding this comment.
Just a note, for quite some commits now it's no longer DetailedError neither ErroMap. Now there is a generic ExposedErrors which expects a type. For API APIError where we set an internal error chain to match and exposed error. Might also be useful to provide the AI with the matching algorithm found on the generic implementation
Summary by CodeRabbit