refactor sweep: lint baseline, tracer hook, bound JOINs, release tooling, "Why gerpo?" docs - #54
Merged
Conversation
The committed flag in txWrap was misspelled across all three adapters (pgx5, pgx4, databasesql). The field is private, so this is not a breaking change. Renamed in code and in docs/architecture/adapters-internals.md.
go.mod has declared go 1.24.0 for a while, but the GitHub Actions matrix was still pinned to 1.21..1.23 and the integration / bench-diff jobs ran on 1.23. README, docs/index.md and docs/architecture/contributing.md also advertised 1.21 as the minimum. Aligning everything on 1.24: - .github/workflows/go.yml: build matrix and codecov pin moved to 1.24. - .github/workflows/integration.yml, bench-diff.yml: setup-go updated to 1.24. - README.md, docs/index.md, docs/architecture/contributing.md: minimum Go version is now 1.24.
Adds .golangci.yml enabling errcheck, govet, ineffassign, misspell, staticcheck, unused and the gofmt formatter, and a "lint" job in .github/workflows/go.yml that runs golangci-lint v2.5.0 on every push and PR. Baseline run surfaced a handful of real issues — fixed in the same commit so the CI is green from day one: - gofmt: re-format three tx.go wrappers and two integration test files (column alignment changed when "commited" was renamed to "committed"). - staticcheck QF1008 in sqlstmt/count.go: drop the unnecessary embedded-field selector when calling reset / clearing the storage. - staticcheck SA4022 in builder_test.go: the &b.field == nil check is always true; replaced with a plain nil check. - staticcheck ST1023 in types/executioncolumns.go: shorten the redundant explicit slice type on var. - ineffassign in repository_test.go (x2) and tests/first_test.go: turn the swallowed errors from helper construction into explicit t.Fatalf/b.Fatal so the test fails fast on a misconfiguration. - misspell in databasesql/tx_test.go: dropped the "commited" reference from the doc comment. - unused: dropped dead fields and types — builder.columns, columnsStorage.model, placeholderDebugger interface — and added a //nolint:unused on optionFn.apply (satisfies the Option interface via dispatch but the linter can't see the type assertion). The config carves out test files from errcheck/staticcheck/ineffassign/ unused (tests legitimately shadow vars and keep helpers around for later use) and waives the QF1008 quick-fix suggestions about embedded selectors as a matter of taste. CLAUDE.md kept untracked; docs/architecture/contributing.md updated to mention the lint job and the new "golangci-lint run" step in the check-in loop.
WithSoftDeletion stored the user-provided SetValueFn as-is and let fmap.Set panic at the first Delete call when the returned value did not match the target field type (e.g. returning time.Time for a *time.Time field). The crash happened deep in the request path, far from the configuration that caused it. soft.go now runs every SetValueFn once during Build through a typed probe (probeSoftDeletionValue) that: - catches any panic inside the callback and turns it into an error; - accepts nil as a valid value for pointer-typed fields; - otherwise checks the returned value with reflect.AssignableTo and reports a precise mismatch (got X, want Y, field path P). The probe runs with context.Background() — SetValueFn is documented as a pure value producer, so a probe call is harmless. Errors land in the existing SoftDeletionBuilder.errors slice and surface from Build(). soft_test.go covers four scenarios: type mismatch, nil-for-pointer, panic-in-probe, happy path, and the pre-existing reject-on-update- protected check. docs/features/soft-delete.md: replaced the "fmap will panic" warning with the new build-time check description.
The legacy LeftJoin/InnerJoin take a callback that returns the JOIN
body verbatim — anything spliced in becomes SQL text, no parameter
binding, no protection against injection. Adds bound counterparts that
mirror WHERE semantics.
API:
h.LeftJoinOn("posts",
"posts.user_id = users.id AND posts.tenant_id = ?", tenantID)
The table reference and the ON-clause body are inlined as text; values
flow through the driver's placeholder mechanism, just like WHERE
arguments. InnerJoinOn behaves the same way.
Implementation:
- sqlstmt/sqlpart/join.go: JoinBuilder now stores a slice of joinPart
(callback or bound) and exposes Values(). JOINOn appends a fixed
text fragment plus its arguments. SQL() concatenates the bodies in
registration order; Reset clears both the join slice and values.
- query/linq/join.go: new joinLeftOn/joinInnerOn kinds; LeftJoinOn /
InnerJoinOn methods append to the entry slice; Apply dispatches to
Join.JOINOn. The legacy LeftJoin/InnerJoin are marked
// Deprecated: in godoc.
- query/persistent.go: PersistentHelper interface gains LeftJoinOn /
InnerJoinOn alongside the legacy methods (which now carry
Deprecated: notices). Implementations forward to the linq builder.
- sqlstmt/{first,list,count,delete}.go: bound JOIN values come before
WHERE values in the final argument list. Extracted mergeArgs
(sqlstmt/args.go) keeps the merging in one place and preserves nil
semantics so existing callers see no behavioural drift.
Tests:
- sqlstmt/sqlpart/join_test.go rewritten for the new API: callback,
bound, mixed, empty-callback skip, reset.
- tests/integration/persistent_query_test.go gains
TestPersistent_LeftJoinOn_BindsArgs (LEFT JOIN restricted by a bound
user_id; only that user keeps post_count=3, others go to 0) and
TestPersistent_InnerJoinOn_FiltersByBoundArg (single user survives
the filter). Both run on all three adapters.
Docs: docs/features/persistent-queries.md gains a "Bound JOIN
parameters" section and demotes the callback variant to a deprecated
"Legacy callback JOIN" with the existing SQL-injection warning.
A bound JOIN arg ($1) and per-request WHERE args ($2..) live in two separate slices joined by mergeArgs in sqlstmt. If the order ever slipped, postgres would silently see e.g. an int landing where it expects a UUID, or vice versa. Adds TestPersistent_LeftJoinOn_ArgOrder_HoldsAcrossWhereAndCount that constructs a configuration where the ordering is verifiable by type: - JOIN bound arg: UUID (users[5].ID). - WHERE: Age >= 25 (int), then Age IN (25, 27, 29) (three ints). - Both shapes go through GetList and Count (different SQL forms but the same mergeArgs path). If JOIN and WHERE values swap, postgres would reject the query with a type mismatch (`invalid input syntax for type uuid`) — so a green test proves $1..$N keep the documented order. Also asserts that the joined user keeps post_count=3 while non-joined users return 0, confirming the bound JOIN argument actually reaches the JOIN clause. Runs on all three adapters via forEachAdapter.
The pgx5/pgx4/databasesql wrappers were ~80% copy-paste: each re-implemented placeholder rewrite, transaction state and the RollbackUnlessCommitted safety net. Three almost-identical bugs (value receivers + missing committed flag clears) had to be fixed in parallel last sprint. Adds executor/adapters/internal — an unexported package containing: - Backend / TxBackend interfaces describing what each driver must provide (Exec/Query/BeginTx; Commit/Rollback for the tx half), - Adapter — the executor.types.DBAdapter implementation that owns placeholder rewrite for ExecContext/QueryContext, - transaction — the wrapper that holds committed / rollbackUnlessCommittedNeeded and exposes the public Tx interface. Each driver package shrinks to a thin Backend + TxBackend pair plus its driver-specific Result/Rows wrappers (which are absent from databasesql because *sql.Rows / sql.Result already match the executor.types interfaces). databasesql/options.go now configures an internal adapterConfig struct, so Option no longer leaks the old dbWrap type into the public API. Question stays the default placeholder format. Tests: - executor/adapters/internal/base_test.go covers placeholder rewrite for both formats, every transaction-lifecycle path (Commit, RollbackUnlessCommitted-after-Commit, RollbackUnlessCommitted- without-Commit, explicit Rollback clearing the safety net, Commit-error keeping the safety net armed) and BeginTx error propagation. Driven by a fakeBackend / fakeTx. - the previous databasesql/tx_test.go is removed — its scenarios are now exercised against the shared transaction state machine and cover all three drivers at once. Integration tests (forEachAdapter) and the lint job stay green. Docs: docs/architecture/adapters-internals.md updated to describe the new shared base, the Backend/TxBackend contracts and the steps for writing a new driver.
Repos that ship to prod usually want to see DB calls in their tracer
of choice (OpenTelemetry, Datadog, …). Doing this externally requires
wrapping the DBAdapter and re-emitting spans for every executor call,
which is awkward. Adds a first-class hook with the same shape every
tracer ends up using.
API:
type SpanEnd func(err error)
type Tracer func(ctx context.Context, op string) (context.Context, SpanEnd)
repo, _ := gerpo.NewBuilder[User]().
DB(adapter, executor.WithTracer(myTracer)).
...
The hook is invoked for every public Executor operation — GetOne,
GetMultiple, Count, InsertOne, Update, Delete — with op names prefixed
"gerpo.". The returned context is propagated downstream so child work
(driver calls, cache reads) lands inside the same span; SpanEnd is
called once with the operation's terminal error.
Implementation:
- executor/options.go: new Tracer type and WithTracer option.
- executor/executor.go: thin startSpan helper short-circuits on a nil
tracer (no allocations / branch when tracing is disabled). Each
public method opens a span at entry and defers end(err); switched to
named return values where needed.
- executor/tracer_test.go: a recordingTracer drives every entry point
and verifies (a) the tracer is invoked exactly once per call,
(b) the op name is correct, (c) the terminal error is reported, and
(d) the no-tracer path is a no-op.
Tracing is opt-in. Logs and metrics intentionally stay external — open
an issue if a dedicated hook becomes useful.
Docs:
- docs/features/tracing.md (new) — adapter recipes for OpenTelemetry
and Datadog, op-name table, "disabled by default" note.
- docs/features/index.md and mkdocs.yml — link to the new page.
Each per-operation helper (GetFirstHelper, GetListHelper, CountHelper,
InsertHelper, UpdateHelper, DeleteHelper) listed its methods inline,
which made it impossible for callers to write reusable middleware-style
helpers (e.g. apply a tenant filter regardless of operation kind).
Adds query/interfaces.go with four small contracts and reuses them via
embedding:
Filterable { Where() types.WhereTarget }
Sortable { OrderBy() types.OrderTarget }
Excludable { Exclude(...); Only(...) }
Pageable[T] { Page(...) GetListHelper[T]; Size(...) GetListHelper[T] }
GetFirstHelper[T] = Filterable + Sortable + Excludable
GetListHelper[T] = Filterable + Sortable + Excludable + Pageable[T]
CountHelper[T] = Filterable
DeleteHelper[T] = Filterable
InsertHelper[T] = Excludable
UpdateHelper[T] = Filterable + Excludable
The aggregate interfaces keep the same names, the same method sets and
the same package, so this is a pure addition to the public API: no
existing call site, type assertion or alternate implementation needs
to change. Concrete helpers (query.GetFirst[T], query.GetList[T], …)
already provide the methods, so they automatically satisfy both the
old aggregate names and the new narrow contracts.
Tests:
- query/interfaces_test.go pins compile-time guarantees that every
concrete helper implements both its aggregate interface and each of
the small contracts. No runtime test needed — embedding is a flat
method-set expansion.
Use case (now possible):
func applyTenant(h query.Filterable, tid uuid.UUID) {
h.Where().Field(...).EQ(tid)
}
repo.GetFirst(ctx, func(m *User, h query.GetFirstHelper[User]) { applyTenant(h, tid) })
repo.Update(ctx, &u, func(m *User, h query.UpdateHelper[User]) { applyTenant(h, tid) })
Docs: docs/features/crud.md gains a tip block introducing the new
contracts with an example. Per-operation pages keep their existing
copy.
pkg.go.dev shows ExampleXxx functions next to the corresponding identifiers. Adds example_test.go in package gerpo_test with snippets for the most common entry points. Examples: - ExampleNewBuilder — minimum builder chain (pgx5). - ExampleRepository_GetFirst — single-row read with ErrNotFound mapping. - ExampleRepository_GetList — filter + ORDER BY + pagination. - ExampleRepository_Insert — InsertHelper.Exclude to defer to DB DEFAULT. - ExampleRepository_Update — Only(...) for column-scoped updates. - ExampleRepository_Delete — Delete with WHERE. - ExampleRepository_Tx — BeginTx + repo.Tx + RollbackUnlessCommitted defer. - ExampleWithSoftDeletion — soft-delete configuration end-to-end. - ExampleWithErrorTransformer — mapping ErrNotFound to a domain error. - ExampleWithTracer — opt-in tracer hook adapter. - ExampleWithCacheStorage — CtxCache wiring. The examples deliberately omit `// Output:` lines because every one of them needs a live database to actually run; the compiler still type- checks them on every `go test ./...`, so they cannot drift from the public API. Two tiny placeholder constructors (exampleRepo, exampleAdapter) keep the snippets niladic without polluting them with boilerplate. docs/index.md mentions that pkg.go.dev now ships runnable examples.
Adds an automated CHANGELOG pipeline built around Conventional Commits
and git-cliff:
- cliff.toml — git-cliff configuration. Maps every Conventional Commit
type to a CHANGELOG section (Features / Bug Fixes / Performance /
Refactor / Documentation / Tests / CI / Build / Misc / Reverts /
BREAKING CHANGES). Uses tag pattern `v[0-9]*` and strips the leading
"v" from the heading.
- CHANGELOG.md — initial render of the existing history under
[Unreleased]. Will be re-rendered with each release.
- scripts/release.sh — local helper that takes a tag (vX.Y.Z) and:
1) refuses to run unless on clean main and the tag is free;
2) git pulls origin/main;
3) regenerates CHANGELOG.md with git-cliff;
4) shows the diff for review;
5) on confirmation, commits the file and creates the annotated tag.
It deliberately does NOT push so the user can `git show <tag>` and
amend before sharing.
- .github/workflows/release.yml — on push of a tag matching v*, runs
`git cliff --latest --strip header` and creates a GitHub Release
with that excerpt as the body. Tags carrying a suffix (e.g.
v1.0.0-rc1) are marked pre-release automatically. No CHANGELOG.md
push-back, no special tokens — only the default GITHUB_TOKEN with
contents:write.
- .github/workflows/commit-lint.yml — on every PR to main, walks the
commits with `git log base..head` and rejects anything that doesn't
start with a recognised Conventional Commits type. Pure shell, no
Node, no extra dependencies.
docs/architecture/contributing.md gains:
- A "Commit style — Conventional Commits" table mapping each allowed
type to its CHANGELOG section, with `!` / "BREAKING CHANGE:"
semantics and examples.
- A "Releasing" section with the runbook (install git-cliff, run
`./scripts/release.sh vX.Y.Z`, then `git push --follow-tags`).
- The PR section now lists five jobs (lint / unit / integration /
bench-diff / commit-lint).
Collects the commands that used to be spread across README, CLAUDE.md and docs/architecture/contributing.md into a single Makefile with self-documenting help. Everything underneath still works unchanged — this is a convenience layer. Targets: - build go build -v ./... - test go test -race ./... - lint golangci-lint run ./... - integration-up docker compose up -d (tests/integration) - integration-down docker compose down - integration go test -tags=integration ./tests/integration/... - integration-full up → run → down in one go - bench Direct vs Gerpo mock benchmarks (5 runs) - bench-report formatted summary via TestCompareDirectVsGerpo - docs-serve mkdocs serve - docs-build mkdocs build --strict - release ./scripts/release.sh $(TAG) - help (default goal) lists the above with docstrings `make help` reads the ## comments directly from the Makefile so new targets show up automatically when added. INTEGRATION_DSN is overridable (default: the compose defaults for the local container). docs/architecture/contributing.md now points at the Makefile in the check-in loop and the release runbook instead of duplicating the go/docker commands.
A new docs page that situates gerpo against the rest of the Go data access ecosystem so visitors do not have to reverse-engineer the niche from the README. Contents: - 30-second pitch and "when to pick" / "when not to pick" sections. - Feature matrix vs GORM, ent, bun, sqlc and sqlx (approach, schema source, type safety, migrations, relations, struct tags, drivers, caching, tracing, hooks, raw SQL escape hatch, line-count, reflection footprint). - Strengths and weaknesses written without marketing — calls out the pre-1.0 surface, deprecated virtual-column API, generic boilerplate and the per-call allocation overhead. - Performance section with the headline numbers from TestCompareDirectVsGerpo and the standard "absorbed by network IO" caveat. - "Closest alternatives — when each fits better" table to tell readers to use a different library when that is the right answer. Wiring: - mkdocs.yml: new top-level entry between "Get started" and "Features". - docs/index.md: a Why-gerpo card slotted into the landing-page grid. - README.md: the docs link in the badge row gains a direct link to the page.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #54 +/- ##
==========================================
+ Coverage 61.17% 63.73% +2.55%
==========================================
Files 54 55 +1
Lines 1955 2010 +55
==========================================
+ Hits 1196 1281 +85
+ Misses 710 678 -32
- Partials 49 51 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The original lint job used golangci/golangci-lint-action@v6, which
explicitly rejects golangci-lint v2:
Error: invalid version string 'v2.5.0', golangci-lint v2 is not
supported by golangci-lint-action v6, you must update to
golangci-lint-action v7.
Bump the action to v7. The pinned linter version (v2.5.0) and the
.golangci.yml config stay unchanged.
Benchmark diff:
|
Benchmark diff:
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
A 14-commit pass through quality, observability, release pipeline and docs.
Nothing breaks the public API; one bug class moves from runtime panic to
build-time error. Every PR check stays green: lint, race-detector unit
tests, integration suite on pgx5/pgx4/database-sql, mock-bench overhead.
Highlights
.golangci.ymlv2 (errcheck/govet/ineffassign/misspell/staticcheck/unused) wired into
.github/workflows/go.yml. All existingfindings fixed in the same commit so CI stays green from day one
(1edc8e1).
executor.WithTracer(fn)plusTracer/SpanEndtypes — OpenTelemetry / Datadog adapters in three lines, noOTel dependency added to gerpo (7f24176).
LeftJoinOn(table, on, args...)andInnerJoinOn(...)route values through driver placeholders. Oldcallback-based
LeftJoin/InnerJoinkeep working, marked// Deprecated:(157fe59). Argument ordering pinned by an extraintegration test (7f737bb).
WithSoftDeletionnow runs a type probe atBuild time — a
SetValueFnreturning the wrong type / panicking iscaught and surfaced as a regular Build error instead of a runtime
panic at first Delete (18e2251).
a tiny
Backend+TxBackendpair plus the driver-specific Result/Rows wrappers; placeholder rewrite, transaction state machine and
RollbackUnlessCommittedsemantics live once inexecutor/adapters/internal/(b95f4f1).Filterable,Sortable,Excludable,Pageable[T]inquery/interfaces.go. Aggregatehelpers (
GetFirstHelper,GetListHelper, …) now embed thosecontracts. Same names, same method-sets — pure addition, no breaking
changes (6d4b507).
cliff.toml+ firstCHANGELOG.mdrenderedfrom history;
scripts/release.sh(wrapped bymake release TAG=...)prepares a tag locally;
.github/workflows/release.ymlbuilds theGitHub Release notes from the same config; new
.github/workflows/commit-lint.ymlenforces Conventional Commits onevery PR (2f9b158).
test,lint,integration*,bench*,docs-*,release—make helpliststhem all (2779502).
method on pkg.go.dev (f3a4a0d). New "Why gerpo?" page comparing
against GORM / ent / bun / sqlc / sqlx with feature matrix and
honest weaknesses (fdadead).