docs+ci+refactor: staff-level README, CI/CD, drop global Redis singleton#1
Conversation
- README: status banner (scaffold), Mermaid target architecture (Redis Streams +
consumer groups + DLQ), Mermaid current code-path sequence, configuration,
roadmap (XACK/XCLAIM, retries+DLQ, OTel/Prom, dockertest), security.
- CI: vet/lint/build (golangci-lint-action@v9 + v2.12).
- Security: gosec v2.26.1, govulncheck on Go stable, Trivy 0.36.0 SARIF; cron.
- Refactor (BEHAVIOR-CHANGING — needs review):
* config.LoadConfig now returns (*Config, error) with wrapped errors instead
of *Config + log.Fatal in the loader (cleaner test seam).
* Replaced global 'var rdb *redis.Client' + sync.Once with App{Redis} returned
from app.New(ctx, cfg). Eliminates global state; tests can construct fresh
Apps.
* app.New does a Ping with a 5s ContextTimeout so misconfigured Redis fails
fast at boot instead of hanging.
* main.go now uses signal.NotifyContext + run() error pattern; defers fire on
every exit path (was using log.Fatal which skipped defers).
* Bumped go-redis from v6 (incompatible / unmaintained) to v8 with
context-aware APIs.
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| v.SetConfigName("config") | ||
| v.SetConfigType("yaml") | ||
| v.AddConfigPath("./config/") | ||
| v.AutomaticEnv() |
There was a problem hiding this comment.
🟡 Missing SetEnvKeyReplacer makes env var overrides for nested config keys silently ineffective
v.AutomaticEnv() is called at config/config.go:33, and the doc comment at line 1-3 explicitly claims environment variables override file values with _ separators (e.g. REDIS_ADDR). However, Viper maps config key redis.addr to env var REDIS.ADDR by default (uppercased with literal .). Without calling v.SetEnvKeyReplacer(strings.NewReplacer(".", "_")), environment variables like REDIS_ADDR, REDIS_PASSWORD, and REDIS_DB will be silently ignored, and the YAML file values will always win. This makes the env override feature broken despite being wired up.
| v.AutomaticEnv() | |
| v.AutomaticEnv() | |
| v.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) |
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Summary
Replace the one-line README with a staff-level scaffold doc; add CI + security workflows; refactor the code from tutorial-style globals into production patterns.
Documentation (low risk)
miniredis/dockertestintegration tests), security considerations.CI / Security (low risk)
.github/workflows/ci.yml:vet,lint(golangci-lint-action@v9 + v2.12),build..github/workflows/security.yml:gosec@v2.26.1,govulncheckon Go stable,aquasecurity/trivy-action@v0.36.0filesystem SARIF; push/PR + weekly cron..golangci.ymlv2 schema:bodyclose, errcheck, gocritic, govet, ineffassign, misspell, prealloc, revive, staticcheck, unconvert, unused.Behavior-changing refactor (HIGHER RISK — please review)
config.LoadConfig()now returns(*Config, error)with wrapped errors instead of*Config+log.Fatalfrom inside the loader. Restores the testable contract.internal/app: removedvar rdb *redis.Client+sync.Onceglobal. NewApp{Redis}constructed viaapp.New(ctx, cfg). The boot-timePINGnow runs under a 5scontext.WithTimeoutso a misconfigured Redis fails fast.main.gonow usessignal.NotifyContext(SIGINT, SIGTERM)+ arun() errorpattern so deferred cleanup runs on every exit path (was relying onlog.Fatal, which skipped defers).github.com/go-redis/redisv6 (+incompatible, no context support, unmaintained) togithub.com/go-redis/redis/v8(context-aware APIs).Review & Testing Checklist for Human
docker run --rm -p 6379:6379 redis:7-alpine, thengo run .from repo root. Expecttaskq started; redis=localhost:6379 db=0followed by clean shutdown onCtrl-C.localhost:6380) — expect a fast failure within ~5 s with a wrapped error likeinit app: redis ping localhost:6380: .... This validates the new ping timeout.taskq/internal/app. The exposed type changed from*redis.Client(v6) to*redis.Client(v8) — same name, different package; method signatures gainedctx context.Contextas the first arg.Notes
go.moddeclaresgo 1.21.0. CI usesgo-version-file: go.modso the CI runner Go version will track that. We can bump to1.22+if you want any of the newer language features.miniredis-backed unit tests (anddockertestfor the future producer/worker loops) is on the roadmap.Link to Devin session: https://app.devin.ai/sessions/475f16f7ca614b56a8f68426fb9485f9
Requested by: @Chetas1