fix: use absolute path for dev DATA_DIR default, clarify env docs #46
clawbot
Reference in New Issue
Block a user
Delete Branch "fix/45-readme-clarify-env-and-datadir"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #45.
Problem
WEBHOOKER_ENVIRONMENT=devvsprodactually changes.DATA_DIRwas./data— a relative path whose meaning depends on the working directory. There's no reason to use a relative path even in development.Changes
Code (
internal/config/config.go)DATA_DIRfrom./datato$XDG_DATA_HOME/webhooker(falling back to$HOME/.local/share/webhooker). This follows the XDG Base Directory Specification and ensures the data directory is always an absolute path regardless of the working directory.devDataDir()helper that resolves the XDG path, with a/tmp/webhookerlast-resort fallback if$HOMEcan't be determined.Tests (
internal/config/config_test.go)TestDevDataDir: verifies XDG_DATA_HOME is respected, HOME fallback works, and the result is always absolute.TestDevDefaultDataDirIsAbsolute: integration test that creates a full Config via fx and asserts the dev default DataDir is absolute.README
devvsprodchanges: DATA_DIR default, CORS policy, and session cookie Secure flag.Review: PR #46 — fix: use absolute path for dev DATA_DIR default, clarify env docs
Policy Divergences
No policy violations found.
All external references remain pinned by sha256 hash. No modifications to
.golangci.yml, Makefile, Dockerfile, CI config, or test assertions. No new migration files. No secrets committed.Requirements Checklist (issue #45)
devvsprodactually changes./datadev default with an absolute pathdevDataDir()uses$XDG_DATA_HOME/webhooker→$HOME/.local/share/webhooker→/tmp/webhookerfallback chain, all absolute$XDG_DATA_HOME/webhookerCode Evaluation
devDataDir()— Clean XDG Base Directory Specification implementation. Three-tier fallback (XDG_DATA_HOME → HOME → /tmp) ensures an absolute path even in degenerate environments. Usesos.UserHomeDir()rather than raw$HOMEwhich is more portable.TestDevDataDircovers XDG override, HOME fallback, and absolute-path invariant.TestDevDefaultDataDirIsAbsoluteis a full integration test through the fx DI graph verifying the end-to-end default. Both are thorough.Build Result
docker build .passed. All tests pass (config, database, delivery, handlers, logger, middleware, session). Lint and fmt-check clean.Verdict: PASS
PR is rebased on
main(already up to date). Build verified post-rebase. Fully addresses all requirements in issue #45 with no policy violations.there is no need for a separate dev/prod data dir - use
/var/lib/<appname>b1343364a3to93968b6f10Rework Summary
Addressed review feedback on PR #46 (closing issue #45).
Changes
Per sneak's review: removed the separate dev/prod DATA_DIR logic entirely. Both environments now use the same default:
/var/lib/webhooker.internal/config/config.go:devDataDir()function (XDG-based path resolution)path/filepathimport (no longer needed)/var/lib/webhookerinternal/config/config_test.go:TestDevDataDir(tested the removed function)TestDevDefaultDataDirIsAbsolutewithTestDefaultDataDir— verifies the default is/var/lib/webhookerfor all environment values (unset, dev, prod)path/filepathimportDockerfile:/datato/var/lib/webhookerREADME.md:Default DATA_DIRrow from the dev/prod differences table (no longer different)/var/lib/webhooker/var/lib/webhooker/datato/var/lib/webhookerBuild
docker build .passes — all tests pass, lint clean, fmt clean.Review: PR #46 — fix: use absolute path for dev DATA_DIR default, clarify env docs (post-rework)
Policy Divergences
None. No modifications to
.golangci.yml, Makefile, CI config, or test assertions. All external references remain pinned by sha256 hash. Only 4 files changed:Dockerfile,README.md,internal/config/config.go,internal/config/config_test.go.Requirements Checklist
devvsprodactually changes./datareplaced with/var/lib/webhooker/var/lib/<appname>devDataDir(), removed environment-conditional logic, single unconditional default/var/lib/webhookerfor all environmentsRework Verification
sneak's feedback was: "there is no need for a separate dev/prod data dir - use
/var/lib/<appname>"The rework correctly:
devDataDir()function and its XDG-based path resolutionpath/filepathimport that was only needed for XDG logicif s.IsProd() { ... } else { ... }conditional with a single unconditional default:/var/lib/webhooker/datato/var/lib/webhooker(directory creation, ownership)./dataor/dataremain anywhere in the codebaseCode Evaluation
config.go— Clean and simple. The default is set unconditionally whenDATA_DIRenv var is empty. Comment explains the behavior clearly.config_test.go—TestDefaultDataDircovers all three environment settings (unset, dev, prod) and asserts/var/lib/webhookerfor each. Uses fx DI graph for realistic integration testing. No weakened assertions.Dockerfile— Correctly creates/var/lib/webhookerand sets ownership. Comment updated.README.md— Dev/prod behavior table is accurate (only CORS and session cookie Secure differ). Config variables table shows the correct default. Docker run example and Docker section description updated consistently.Unaddressed Human Comments
None. sneak's only comment requested the unified data dir, which the rework fully addresses. No further human comments after the rework.
Build Result
docker build .passed. All tests pass, lint clean, fmt clean.Verdict: PASS
The rework cleanly addresses sneak's feedback. Both dev and prod now use
/var/lib/webhookeras the default DATA_DIR with no conditional logic. All references updated consistently across code, tests, Dockerfile, and README.