refactor(output): make slog silence a level rather than a convention - #133
Open
Ilyes512 wants to merge 1 commit into
Open
refactor(output): make slog silence a level rather than a convention#133Ilyes512 wants to merge 1 commit into
Ilyes512 wants to merge 1 commit into
Conversation
Logger setup happened in two files and neither call was the whole story. NewApp installed a text handler on os.Stderr at Info level; PersistentPreRunE adjusted the level and replaced the handler only when both --debug and -o json were given. That left three problems. The documented "silent on a normal run" promise held only because every log point in the tree happens to be slog.Debug — the first slog.Info anyone reached for would print on every run. The far more common --debug alone kept the os.Stderr handler, so debug output escaped the buffers a test points the command at. And the format coupling lived as a nested conditional in a command file rather than as a property of the logger. output.SetupLogger is now the only place a handler is built, and LevelSilent — above every level slog defines — is what a run without --debug gets. It is called twice: once by NewApp with os.Stderr, since cobra has not parsed the flags yet and a failure before that still needs a logger, and once by PersistentPreRunE with the command's own stderr and the resolved flags. The unused WithDebug / WithHandler / HandlerFactory option plumbing goes with it; nothing outside its own doc comment ever called it. template validate no longer forces the level to Debug. That line was undocumented and untested, and made the one command spray diagnostics at a user who had not asked for them — which is exactly the invariant above. Closes #114
Ilyes512
marked this pull request as ready for review
September 2, 2026 22:20
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
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.
Logger setup happened in two files and neither call was the whole story.
NewApp installed a text handler on os.Stderr at Info level; PersistentPreRunE
adjusted the level and replaced the handler only when both --debug and
-o json were given.
That left three problems. The documented "silent on a normal run" promise held
only because every log point in the tree happens to be slog.Debug — the first
slog.Info anyone reached for would print on every run. The far more common
--debug alone kept the os.Stderr handler, so debug output escaped the buffers
a test points the command at. And the format coupling lived as a nested
conditional in a command file rather than as a property of the logger.
output.SetupLogger is now the only place a handler is built, and LevelSilent —
above every level slog defines — is what a run without --debug gets. It is
called twice: once by NewApp with os.Stderr, since cobra has not parsed the
flags yet and a failure before that still needs a logger, and once by
PersistentPreRunE with the command's own stderr and the resolved flags.
The unused WithDebug / WithHandler / HandlerFactory option plumbing goes with
it; nothing outside its own doc comment ever called it.
template validate no longer forces the level to Debug. That line was
undocumented and untested, and made the one command spray diagnostics at a
user who had not asked for them — which is exactly the invariant above.
Closes #114
Stack created with GitHub Stacks CLI • Give Feedback 💬