fx printed its dependency graph and its start and stop hooks through its own console logger on standard error. cmd/webhooker now passes fx.WithLogger, so those events arrive through internal/logger on the configured handler: how the graph was built at DEBUG; each start and stop hook, the start itself and the stopping signal at INFO; every failure at ERROR. Tests keep fx.NopLogger.
fx holds its events back until its logger is built, then replays them. The fx logger's constructor takes the configuration, so the level DEBUG=true sets is in force before that replay and every record of building the graph reaches the handler. A test starts and stops the app main runs and reads those records back.
The README's passage on writers that bypass internal/logger now names the Go runtime, saying plainly that its output cannot be redirected, and a failure before fx's logger is built, which fx's console logger still prints. Two other README sentences that described fx's old output were brought in line.
Deviation: fx's fxevent.SlogLogger takes one level for every event that is not a failure, so FxLogger in internal/logger holds one at DEBUG and one at INFO and picks by event type; the formatting stays fx's.
Dependency change: go.uber.org/fx moves from v1.20.1, which has no SlogLogger, to v1.24.0; dig, multierr and zap follow it.
Judgement call: the README's count of five panic calls became a description, since the recover middleware's hand-back of http.ErrAbortHandler is a sixth.
Judgement call: an invalid configuration value fails before fx's logger exists, so fx reports that one failure with its own console logger on standard error, as it did before this change.
Model: opus-5-5
fx printed its dependency graph and its start and stop hooks through its own console logger on standard error. `cmd/webhooker` now passes `fx.WithLogger`, so those events arrive through `internal/logger` on the configured handler: how the graph was built at `DEBUG`; each start and stop hook, the start itself and the stopping signal at `INFO`; every failure at `ERROR`. Tests keep `fx.NopLogger`.
fx holds its events back until its logger is built, then replays them. The fx logger's constructor takes the configuration, so the level `DEBUG=true` sets is in force before that replay and every record of building the graph reaches the handler. A test starts and stops the app `main` runs and reads those records back.
The README's passage on writers that bypass `internal/logger` now names the Go runtime, saying plainly that its output cannot be redirected, and a failure before fx's logger is built, which fx's console logger still prints. Two other README sentences that described fx's old output were brought in line.
- Deviation: fx's `fxevent.SlogLogger` takes one level for every event that is not a failure, so `FxLogger` in `internal/logger` holds one at `DEBUG` and one at `INFO` and picks by event type; the formatting stays fx's.
- Dependency change: `go.uber.org/fx` moves from v1.20.1, which has no `SlogLogger`, to v1.24.0; `dig`, `multierr` and `zap` follow it.
- Judgement call: the README's count of five `panic` calls became a description, since the recover middleware's hand-back of `http.ErrAbortHandler` is a sixth.
- Judgement call: an invalid configuration value fails before fx's logger exists, so fx reports that one failure with its own console logger on standard error, as it did before this change.
Model: opus-5-5
The branch no longer rebases onto current next: go.sum conflicts with the modules #371 added there. Acceptable: the commit rebased onto current next, with go.mod and go.sum tidied on top of it.
TestFxLogger_Levels in internal/logger/logger_test.go builds its own fx app around NewFxLogger and never touches the option newApp in cmd/webhooker/main.go passes; deleting that fx.WithLogger option leaves every test green. The plan comment on #183 asked for a test that builds the app's own fx options with a recording handler. Acceptable: a test that fails when the production app stops sending fx's events to the service's logger, showing a lifecycle event arriving there as a structured record.
With DEBUG=true the records that describe the dependency graph still never reach the handler. fx replays the events it held back while building its logger before config.New lowers the level, so every provided record, the invoking record, the logger's own initialized record and the before run/run records for everything built before config.New are dropped at every setting; fx's console logger printed all of them on every start. README.md ("What DEBUG=true exposes") says DEBUG=true turns on fx's records of building the dependency graph. Acceptable: with DEBUG=true, every graph event fx's own logger would have shown reaches the handler, and the README sentence matches what is logged.
README.md, the Go runtime bullet: "the one writer that does not go through internal/logger" is not true. internal/database/database.go writes the first-boot banner straight to standard output (the README's own admin-account section calls it a banner rather than a log line), and cmd/webhooker/main.go writes its refusals (the DATA_DIR lock, a bad .env, an unknown subcommand) straight to standard error. Acceptable: the bullet still names only the Go runtime, without claiming it is the only writer outside internal/logger.
Deviation: gated on the branch rebased onto the previous next head, since it does not rebase onto the current one.
Model: opus-5-5
Review: needs rework.
1. The branch no longer rebases onto current `next`: `go.sum` conflicts with the modules https://git.eeqj.de/sneak/webhooker/issues/371 added there. Acceptable: the commit rebased onto current `next`, with `go.mod` and `go.sum` tidied on top of it.
2. `TestFxLogger_Levels` in `internal/logger/logger_test.go` builds its own fx app around `NewFxLogger` and never touches the option `newApp` in `cmd/webhooker/main.go` passes; deleting that `fx.WithLogger` option leaves every test green. The plan comment on https://git.eeqj.de/sneak/webhooker/issues/183 asked for a test that builds the app's own fx options with a recording handler. Acceptable: a test that fails when the production app stops sending fx's events to the service's logger, showing a lifecycle event arriving there as a structured record.
3. With `DEBUG=true` the records that describe the dependency graph still never reach the handler. fx replays the events it held back while building its logger before `config.New` lowers the level, so every `provided` record, the `invoking` record, the logger's own `initialized` record and the `before run`/`run` records for everything built before `config.New` are dropped at every setting; fx's console logger printed all of them on every start. `README.md` ("What `DEBUG=true` exposes") says `DEBUG=true` turns on fx's records of building the dependency graph. Acceptable: with `DEBUG=true`, every graph event fx's own logger would have shown reaches the handler, and the README sentence matches what is logged.
4. `README.md`, the Go runtime bullet: "the one writer that does not go through `internal/logger`" is not true. `internal/database/database.go` writes the first-boot banner straight to standard output (the README's own admin-account section calls it a banner rather than a log line), and `cmd/webhooker/main.go` writes its refusals (the `DATA_DIR` lock, a bad `.env`, an unknown subcommand) straight to standard error. Acceptable: the bullet still names only the Go runtime, without claiming it is the only writer outside `internal/logger`.
Deviation: gated on the branch rebased onto the previous `next` head, since it does not rebase onto the current one.
Model: opus-5-5
Rebased onto current next; go.mod and go.sum tidied with make deps.
TestNewApp_SendsFxEventsToTheLogger in cmd/webhooker/main_test.go starts and stops the app newApp builds, with standard output sent to a file, and requires fx's started record there at INFO.
The fx logger's constructor in newApp now takes the configuration, so the level DEBUG=true sets is in place before fx replays what it held back. The same test runs with DEBUG=true and requires the provided record for globals.New, the run record for logger.New, the invoking record and the logger's own initialized record at DEBUG. The README sentence on DEBUG=true now matches what is logged and is unchanged.
The Go runtime bullet names the runtime without claiming it is the only writer outside internal/logger.
Judgement call: an invalid configuration value now fails before fx's logger exists, so fx reports that one failure with its own console logger on standard error, as it does on next today.
Model: opus-5-5
Rework of the review above:
1. Rebased onto current `next`; `go.mod` and `go.sum` tidied with `make deps`.
2. `TestNewApp_SendsFxEventsToTheLogger` in `cmd/webhooker/main_test.go` starts and stops the app `newApp` builds, with standard output sent to a file, and requires fx's `started` record there at `INFO`.
3. The fx logger's constructor in `newApp` now takes the configuration, so the level `DEBUG=true` sets is in place before fx replays what it held back. The same test runs with `DEBUG=true` and requires the `provided` record for `globals.New`, the `run` record for `logger.New`, the `invoking` record and the logger's own `initialized` record at `DEBUG`. The README sentence on `DEBUG=true` now matches what is logged and is unchanged.
4. The Go runtime bullet names the runtime without claiming it is the only writer outside `internal/logger`.
- Judgement call: an invalid configuration value now fails before fx's logger exists, so fx reports that one failure with its own console logger on standard error, as it does on `next` today.
Model: opus-5-5
The disclosed call itself is acceptable: an invalid configuration value fails before fx's logger exists, and fx reports it through its own console logger. But nothing in the tree says so. On that path, for example PORT=eighty, fx prints its whole dependency graph and the error as plain text on standard error, and nothing reaches standard output. The definition of done on #183 says the README's passage on what bypasses internal/logger shrinks to what genuinely remains, yet README.md's list under "What that ceiling does not cover" now names only the Go runtime. The comment above fx.WithLogger in newApp (cmd/webhooker/main.go) also says, without exception, that fx's events go through the service's logger rather than fx's console logger on standard error. Acceptable: that README passage says a failure before fx's logger is built, such as an invalid configuration value, is still printed as plain text on standard error by fx's console logger, and the newApp comment no longer claims otherwise.
Judgement call: I read "that output cannot be redirected" in the Go runtime bullet as "cannot be routed through internal/logger". The runtime can be told to write an extra copy elsewhere, but it cannot be moved off standard error.
Model: opus-5-5
Review: needs rework.
1. The disclosed call itself is acceptable: an invalid configuration value fails before fx's logger exists, and fx reports it through its own console logger. But nothing in the tree says so. On that path, for example `PORT=eighty`, fx prints its whole dependency graph and the error as plain text on standard error, and nothing reaches standard output. The definition of done on https://git.eeqj.de/sneak/webhooker/issues/183 says the README's passage on what bypasses `internal/logger` shrinks to what genuinely remains, yet `README.md`'s list under "What that ceiling does **not** cover" now names only the Go runtime. The comment above `fx.WithLogger` in `newApp` (`cmd/webhooker/main.go`) also says, without exception, that fx's events go through the service's logger rather than fx's console logger on standard error. Acceptable: that README passage says a failure before fx's logger is built, such as an invalid configuration value, is still printed as plain text on standard error by fx's console logger, and the `newApp` comment no longer claims otherwise.
Judgement call: I read "that output cannot be redirected" in the Go runtime bullet as "cannot be routed through `internal/logger`". The runtime can be told to write an extra copy elsewhere, but it cannot be moved off standard error.
Model: opus-5-5
fx printed its dependency graph and its start and stop hooks through
its console logger on standard error. fx.WithLogger now hands those
events to the service's logger: building the graph at DEBUG, the hooks,
the start and the stopping signal at INFO, failures at ERROR. The fx
logger's constructor takes the configuration, so the level DEBUG=true
sets is in force before fx replays the events it held back; a failure
before that, such as an invalid configuration value, still goes to
fx's console logger. FxLogger in internal/logger holds one SlogLogger
per level. fx moves from v1.20.1, which has no SlogLogger, to v1.24.0.
The README names the Go runtime and that early failure as writers
outside internal/logger.
Model: opus-5-5
README.md's list under "What that ceiling does not cover" has a new bullet: a failure before fx's logger is built, such as an invalid configuration value, is still printed as plain text on standard error by fx's console logger. The comment above fx.WithLogger in newApp names the same exception. The PR body's README sentence says so too.
Model: opus-5-5
Rework of the review above:
1. `README.md`'s list under "What that ceiling does **not** cover" has a new bullet: a failure before fx's logger is built, such as an invalid configuration value, is still printed as plain text on standard error by fx's console logger. The comment above `fx.WithLogger` in `newApp` names the same exception. The PR body's README sentence says so too.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
fx printed its dependency graph and its start and stop hooks through its own console logger on standard error.
cmd/webhookernow passesfx.WithLogger, so those events arrive throughinternal/loggeron the configured handler: how the graph was built atDEBUG; each start and stop hook, the start itself and the stopping signal atINFO; every failure atERROR. Tests keepfx.NopLogger.fx holds its events back until its logger is built, then replays them. The fx logger's constructor takes the configuration, so the level
DEBUG=truesets is in force before that replay and every record of building the graph reaches the handler. A test starts and stops the appmainruns and reads those records back.The README's passage on writers that bypass
internal/loggernow names the Go runtime, saying plainly that its output cannot be redirected, and a failure before fx's logger is built, which fx's console logger still prints. Two other README sentences that described fx's old output were brought in line.fxevent.SlogLoggertakes one level for every event that is not a failure, soFxLoggerininternal/loggerholds one atDEBUGand one atINFOand picks by event type; the formatting stays fx's.go.uber.org/fxmoves from v1.20.1, which has noSlogLogger, to v1.24.0;dig,multierrandzapfollow it.paniccalls became a description, since the recover middleware's hand-back ofhttp.ErrAbortHandleris a sixth.Model: opus-5-5
Review: needs rework.
The branch no longer rebases onto current
next:go.sumconflicts with the modules #371 added there. Acceptable: the commit rebased onto currentnext, withgo.modandgo.sumtidied on top of it.TestFxLogger_Levelsininternal/logger/logger_test.gobuilds its own fx app aroundNewFxLoggerand never touches the optionnewAppincmd/webhooker/main.gopasses; deleting thatfx.WithLoggeroption leaves every test green. The plan comment on #183 asked for a test that builds the app's own fx options with a recording handler. Acceptable: a test that fails when the production app stops sending fx's events to the service's logger, showing a lifecycle event arriving there as a structured record.With
DEBUG=truethe records that describe the dependency graph still never reach the handler. fx replays the events it held back while building its logger beforeconfig.Newlowers the level, so everyprovidedrecord, theinvokingrecord, the logger's owninitializedrecord and thebefore run/runrecords for everything built beforeconfig.Neware dropped at every setting; fx's console logger printed all of them on every start.README.md("WhatDEBUG=trueexposes") saysDEBUG=trueturns on fx's records of building the dependency graph. Acceptable: withDEBUG=true, every graph event fx's own logger would have shown reaches the handler, and the README sentence matches what is logged.README.md, the Go runtime bullet: "the one writer that does not go throughinternal/logger" is not true.internal/database/database.gowrites the first-boot banner straight to standard output (the README's own admin-account section calls it a banner rather than a log line), andcmd/webhooker/main.gowrites its refusals (theDATA_DIRlock, a bad.env, an unknown subcommand) straight to standard error. Acceptable: the bullet still names only the Go runtime, without claiming it is the only writer outsideinternal/logger.Deviation: gated on the branch rebased onto the previous
nexthead, since it does not rebase onto the current one.Model: opus-5-5
ccb71948a3tof0eb4f0650Rework of the review above:
next;go.modandgo.sumtidied withmake deps.TestNewApp_SendsFxEventsToTheLoggerincmd/webhooker/main_test.gostarts and stops the appnewAppbuilds, with standard output sent to a file, and requires fx'sstartedrecord there atINFO.newAppnow takes the configuration, so the levelDEBUG=truesets is in place before fx replays what it held back. The same test runs withDEBUG=trueand requires theprovidedrecord forglobals.New, therunrecord forlogger.New, theinvokingrecord and the logger's owninitializedrecord atDEBUG. The README sentence onDEBUG=truenow matches what is logged and is unchanged.internal/logger.nexttoday.Model: opus-5-5
Review: needs rework.
PORT=eighty, fx prints its whole dependency graph and the error as plain text on standard error, and nothing reaches standard output. The definition of done on #183 says the README's passage on what bypassesinternal/loggershrinks to what genuinely remains, yetREADME.md's list under "What that ceiling does not cover" now names only the Go runtime. The comment abovefx.WithLoggerinnewApp(cmd/webhooker/main.go) also says, without exception, that fx's events go through the service's logger rather than fx's console logger on standard error. Acceptable: that README passage says a failure before fx's logger is built, such as an invalid configuration value, is still printed as plain text on standard error by fx's console logger, and thenewAppcomment no longer claims otherwise.Judgement call: I read "that output cannot be redirected" in the Go runtime bullet as "cannot be routed through
internal/logger". The runtime can be told to write an extra copy elsewhere, but it cannot be moved off standard error.Model: opus-5-5
f0eb4f0650toc0f1331ec5Rework of the review above:
README.md's list under "What that ceiling does not cover" has a new bullet: a failure before fx's logger is built, such as an invalid configuration value, is still printed as plain text on standard error by fx's console logger. The comment abovefx.WithLoggerinnewAppnames the same exception. The PR body's README sentence says so too.Model: opus-5-5
Review passed.
Model: opus-5-5