Conversation
5.4-mini pass. Build + tests clean. - errors.go: restructured (+176 lines new logic), aligning with RFC error shape requirements - log.go: small polish Co-Authored-By: Virgil <virgil@lethean.io>
Removes github.com/stretchr/testify from go.mod/go.sum; rewrites errors_test.go to stdlib t.Fatalf patterns. go mod tidy + go vet + go test all clean (GOWORK=off). Closes tasks.lthn.sh/view.php?id=803 Co-authored-by: Codex <noreply@openai.com> Via-codex-lane: Cladius-solo dispatch (Mac codex CLI)
go-log is foundational — core.* helpers are downstream of it, so log.go, errors.go, and their tests must use stdlib fmt/os/os-user/ strings/sync/errors directly. Added `// Note:` annotations on each import documenting that core is downstream and self-dependency would create an import cycle. Verification: go build ./... passes. Closes tasks.lthn.sh/view.php?id=677 Co-authored-by: Codex <noreply@openai.com>
Dropped the stale `core` segment per RFC, aligning with graduated
repos: dappco.re/go/{name}. No *.go self-imports existed — single-line
go.mod change. `go build ./...` passes.
Closes tasks.lthn.sh/view.php?id=800
Co-authored-by: Codex <noreply@openai.com>
Verification: task -d tests/cli/log + go test all pass. Closes tasks.lthn.sh/view.php?id=679 Co-authored-by: Codex <noreply@openai.com>
Removed direct fmt + strings imports. Substitutions: - %q formatting → strconv.Quote - fmt.Fprintf final write → io.WriteString - strings.NewReplacer → inline control-character normalisation - %v rendering → slog.AnyValue(...).String() go.mod intentionally not modified — module does not require dappco.re/go/core, so no need to add it as a dep just for this purge. Build + race tests green. Co-authored-by: Codex <noreply@openai.com> Closes tasks.lthn.sh/view.php?id=801
- Annotated remaining test-only bytes + strings imports - Replaced errors.New with NewError in recovery test - Removed os import by changing nil output test to assert fallback behaviour without referencing os.Stderr Race PASS. Co-authored-by: Codex <noreply@openai.com> Closes tasks.lthn.sh/view.php?id=678
…(#677) Updated annotations to AX-6 format. errors and strings are structural stdlib primitives; core.E and core.* helpers are downstream of go-log. Closes tasks.lthn.sh/view.php?id=677 Co-authored-by: Codex <noreply@openai.com>
#408: Username() no longer calls user.Current() — falls back to USER/USERNAME env. os/user import removed. RFC §Username updated to match. #409: AX-6 circular-dependency exception notes added to log.go + errors.go documenting why structured logging cannot import core (core depends on log). Verification: go test ./... passed. Co-authored-by: Codex <noreply@openai.com> Closes tasks.lthn.sh/view.php?id=408 Closes tasks.lthn.sh/view.php?id=409
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull request overview
Routine dev→main sync PR that updates the go-log module/API surface and adds a CLI-based driver test to exercise logging + structured error behavior outside the unit test harness.
Changes:
- Add an AX-10 CLI “driver” (
tests/cli/log) plus Taskfile entry to validate logger output and structured error helpers. - Update logger formatting/concurrency (escape handling, string quoting, per-logger write mutex) and refine
Username()to avoid account DB lookups. - Refactor structured-error introspection to traverse joined error “trees”, and remove external test dependencies (e.g.,
testify) alongside a module path change todappco.re/go/log.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/cli/log/main.go | New CLI driver that asserts log rendering, redaction, newline escaping, error metadata, default logger, and rotation hook behavior. |
| tests/cli/log/Taskfile.yaml | Adds runnable Task targets to build/run the CLI driver in an isolated cache environment. |
| specs/RFC.md | Updates Username() specification to match the new env-only lookup behavior. |
| log_test.go | Aligns tests with updated error creation and fallback output semantics. |
| log.go | Adds write serialization and changes key/value stringification + newline escaping; updates Username() implementation. |
| go.sum | Removes prior dependency checksums (now empty). |
| go.mod | Renames module path to dappco.re/go/log and drops test dependencies. |
| errors_test.go | Removes testify usage and replaces assertions with stdlib testing patterns; adds slices comparisons. |
| errors.go | Refactors error-chain helpers to traverse error trees (including errors.Join) via walkErrorTree. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err == nil { | ||
| return | ||
| } | ||
| if !visit(err) { | ||
| return | ||
| } | ||
| switch current := any(err).(type) { | ||
| case multiUnwrapper: | ||
| for _, child := range current.Unwrap() { | ||
| walkErrorTree(child, visit) | ||
| } | ||
| case singleUnwrapper: | ||
| walkErrorTree(current.Unwrap(), visit) | ||
| } |
| switch current := any(err).(type) { | ||
| case multiUnwrapper: | ||
| children := current.Unwrap() | ||
| if len(children) == 0 { | ||
| return err | ||
| } | ||
| return Root(children[0]) | ||
| case singleUnwrapper: | ||
| unwrapped := current.Unwrap() | ||
| if unwrapped == nil { | ||
| return err | ||
| } | ||
| err = unwrapped | ||
| return Root(unwrapped) | ||
| default: |
| module dappco.re/go/log | ||
|
|
||
| go 1.26.0 |
| // AllOps returns an iterator over all operational contexts in the error chain. | ||
| // It traverses the error tree using errors.Unwrap. | ||
| // | ||
| // for op := range log.AllOps(err) { /* "api.Call" → "db.Query" → ... */ } | ||
| func AllOps(err error) iter.Seq[string] { |
Routine dev→main PR opened by Hephaestus PR-cadence task.
This PR exists to:
ahead_by: 9 commit(s) (per gh api compare)If CodeRabbit clears with no blocking comments, this PR is eligible for
gh pr merge --merge(real merge commit, no force-push). Conflicts and review feedback should be addressed on the dev branch before merge.Co-authored-by: Hephaestus hephaestus@cladius