Skip to content

feat(logging): unify logger injection via config.WithLogger (#52) - #56

Merged
harrydayexe merged 8 commits into
mainfrom
logging-review
Jun 8, 2026
Merged

harrydayexe merged 8 commits into
mainfrom
logging-review

Conversation

@harrydayexe

@harrydayexe harrydayexe commented Jun 8, 2026 •

Copy link
Copy Markdown
Owner

Standardise structured logging across all library packages on the functional-options pattern used throughout the rest of the codebase.

Previously pkg/server took a positional *slog.Logger while pkg/generator, pkg/watcher, pkg/outputter, and pkg/parser resolved slog.Default() internally with no caller injection.

Changes:

  • Add config.Logger named type and WithLogger(*slog.Logger) BaseOption
  • WatcherOption now embeds BaseOption (consistent with BaseServerOption and GeneratorOption); WithBaseWatcherOption helper added
  • All consumer structs (Generator, Server, HandlerConfig, Watcher, DirectoryWriter, Parser) embed config.Logger; default to slog.Default() when no logger option is supplied
  • parser.WithLogger added to the parser's own option system; Generator propagates its resolved logger into the parser it creates internally
  • server.New and server.Handler keep their positional *slog.Logger parameter but mark it deprecated (v3.0.0 will remove it); WithLogger option takes precedence over the positional arg
  • Server propagates its resolved logger into the internal generator and handler it constructs
  • CLI (internal/server) migrated to the new pattern: passes nil for the deprecated positional arg and supplies config.WithLogger via cfg.Server
  • Documentation updated across all affected packages and README
  • Injection tests added to all packages; internal test for generator propagation via pkg/server/logger_test.go

Phase 2 (removing the positional logger entirely) tracked in issue #55.

Closes #52

Changelog (#56)

✨ New Features

🐛 Bug Fixes

  • (watcher) use embedded logger in Remove event handler
  • (server) avoid mutating caller's slice backing array in New

📚 Documentation

  • update logger injection examples to use As*Option methods
  • fix small error in example

❓ Uncategorised!

  • Merge branch 'main' into logging-review

Standardise structured logging across all library packages on the
functional-options pattern used throughout the rest of the codebase.

Previously pkg/server took a positional *slog.Logger while pkg/generator,
pkg/watcher, pkg/outputter, and pkg/parser resolved slog.Default()
internally with no caller injection.

Changes:
- Add config.Logger named type and WithLogger(*slog.Logger) BaseOption
- WatcherOption now embeds BaseOption (consistent with BaseServerOption
  and GeneratorOption); WithBaseWatcherOption helper added
- All consumer structs (Generator, Server, HandlerConfig, Watcher,
  DirectoryWriter, Parser) embed config.Logger; default to slog.Default()
  when no logger option is supplied
- parser.WithLogger added to the parser's own option system; Generator
  propagates its resolved logger into the parser it creates internally
- server.New and server.Handler keep their positional *slog.Logger
  parameter but mark it deprecated (v3.0.0 will remove it); WithLogger
  option takes precedence over the positional arg
- Server propagates its resolved logger into the internal generator and
  handler it constructs
- CLI (internal/server) migrated to the new pattern: passes nil for the
  deprecated positional arg and supplies config.WithLogger via cfg.Server
- Documentation updated across all affected packages and README
- Injection tests added to all packages; internal test for generator
  propagation via pkg/server/logger_test.go

Phase 2 (removing the positional logger entirely) tracked in issue #55.

Closes #52
The subdirectory-deletion handler merged in from #54 still referenced
the old local `logger` variable that was removed when config.Logger was
embedded on Watcher. Replace both call sites with w.Logger.Logger.
Logger now implements config.Option[BaseOption] via an AsOption()
method, matching the existing BlogRoot pattern. This lets any component
forward its resolved logger to sub-components (generator, handler,
watcher) with a single call instead of reaching into the underlying
*slog.Logger field.

- pkg/config: add Logger.AsOption(), godoc for Option interface and
  Logger type, and a matching doc comment on BlogRoot.AsOption()
- pkg/server: use srv.Logger.AsOption() at both forwarding sites
- internal/server: wire watcher to the server's resolved logger
Introduces BaseOption.AsGeneratorOption(), AsWatcherOption(), and
AsServerOption() methods as the canonical way to lift a BaseOption into
a specialised option type, replacing the standalone wrapper functions.

WithBaseWatcherOption is removed entirely (not yet shipped).
WithBaseOption is marked deprecated for removal at v3.0.0 (#57).
All call sites, doc examples, and test comments are updated to use the
new method form.
README and newly added tests were still using the deprecated
WithBaseOption/WithBaseWatcherOption wrappers and old BaseServerOption
struct-literal syntax, added before the As*Option pattern landed.
Use make+append instead of appending directly to opts.Gen, which would
write into the caller's backing array if spare capacity existed.
@harrydayexe
harrydayexe merged commit d4d1a7c into main Jun 8, 2026
4 checks passed
@harrydayexe
harrydayexe deleted the logging-review branch June 8, 2026 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review logging strategy across the library

1 participant