docs(logger): document intentional Log-Level Quad-Function Pattern as stable#4880
Merged
Conversation
4 tasks
Address duplicate code issue by explicitly documenting that the four-level wrapper blocks in file_logger.go, markdown_logger.go, and server_file_logger.go are intentional idiomatic Go API design. - common.go: add statement that log levels are stable and not expected to grow; explain why code generation was rejected; add step-by-step instructions for the rare case a new level must be added (satisfies the implementation checklist in the issue) - file_logger.go, markdown_logger.go, server_file_logger.go: add cross-reference comments on each var block pointing to common.go for the rationale and update instructions Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/22bbbaec-2f69-4ff0-bf02-5b672d30fb85 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Refactor duplicate logger level wrapper functions
docs(logger): document intentional Log-Level Quad-Function Pattern as stable
Apr 30, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Documents the intentional “Log-Level Quad-Function Pattern” used by the logger package, clarifying that the Info/Warn/Error/Debug wrapper sets are a stable, deliberate API choice and providing explicit maintenance steps if a new level is ever introduced.
Changes:
- Expanded
internal/logger/common.godocumentation to explain why the 4-level wrapper pattern is intentional/stable and why code generation was rejected. - Added explicit, numbered “if a new LogLevel is added” update steps, including the existing wrapper-coverage test.
- Added short cross-reference comments above the per-file wrapper
varblocks in the three logger implementations to point contributors tocommon.go.
Show a summary per file
| File | Description |
|---|---|
| internal/logger/common.go | Extends the doc block with rationale, stability expectations, and concrete update instructions (including the wrapper coverage test). |
| internal/logger/file_logger.go | Adds a cross-reference comment above the level-bound closure var block pointing to the pattern documentation. |
| internal/logger/markdown_logger.go | Adds the same cross-reference comment above the markdown wrapper var block. |
| internal/logger/server_file_logger.go | Adds the same cross-reference comment above the server wrapper var block. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 0
This was referenced Apr 30, 2026
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.
Three logger files each repeat a near-identical 4-function block (
LogInfo/Warn/Error/Debugvariants) that wraps closures built bymakeLevelLogger/makeServerLevelLogger. The pattern is correct and idiomatic Go — the duplication is intentional — but nothing in the code communicated that decision to future contributors.Changes
common.go— Expanded the "Log-Level Quad-Function Pattern" doc block to:log_level_wrappers_test.go)file_logger.go/markdown_logger.go/server_file_logger.go— Added a 4-line cross-reference comment on eachvarblock directing contributors tocommon.gofor rationale and update stepsWarning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
example.com/tmp/go-build3939558618/b513/launcher.test /tmp/go-build3939558618/b513/launcher.test -test.testlogfile=/tmp/go-build3939558618/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build3939558618/b437/vet.cfg w6WVsrdoR 2085962/b314/ x_amd64/vet /tmp/go-build286/opt/hostedtoolcache/go/1.25.9/x64/pkg/tool/linux_amd64/vet -O2 -fno-stack-prote-bool x_amd64/vet 2085�� olang.org/grpc@v-errorsas olang.org/grpc@v-ifaceassert x_amd64/vet . --gdwarf2 --64 x_amd64/vet(dns block)invalid-host-that-does-not-exist-12345.com/tmp/go-build3939558618/b495/config.test /tmp/go-build3939558618/b495/config.test -test.testlogfile=/tmp/go-build3939558618/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build3939558618/b399/vet.cfg g_.a -I x_amd64/vet --gdwarf-5 gr/stdr -o x_amd64/vet -W Qhn-DafK5 cfg x_amd64/vet . --gdwarf2 --64 x_amd64/vet(dns block)nonexistent.local/tmp/go-build3939558618/b513/launcher.test /tmp/go-build3939558618/b513/launcher.test -test.testlogfile=/tmp/go-build3939558618/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build3939558618/b437/vet.cfg w6WVsrdoR 2085962/b314/ x_amd64/vet /tmp/go-build286/opt/hostedtoolcache/go/1.25.9/x64/pkg/tool/linux_amd64/vet -O2 -fno-stack-prote-bool x_amd64/vet 2085�� olang.org/grpc@v-errorsas olang.org/grpc@v-ifaceassert x_amd64/vet . --gdwarf2 --64 x_amd64/vet(dns block)slow.example.com/tmp/go-build3939558618/b513/launcher.test /tmp/go-build3939558618/b513/launcher.test -test.testlogfile=/tmp/go-build3939558618/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build3939558618/b437/vet.cfg w6WVsrdoR 2085962/b314/ x_amd64/vet /tmp/go-build286/opt/hostedtoolcache/go/1.25.9/x64/pkg/tool/linux_amd64/vet -O2 -fno-stack-prote-bool x_amd64/vet 2085�� olang.org/grpc@v-errorsas olang.org/grpc@v-ifaceassert x_amd64/vet . --gdwarf2 --64 x_amd64/vet(dns block)this-host-does-not-exist-12345.com/tmp/go-build3939558618/b522/mcp.test /tmp/go-build3939558618/b522/mcp.test -test.testlogfile=/tmp/go-build3939558618/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s rtcf�� vel-wrapper 64/src/internal/-ifaceassert x_amd64/vet --gdwarf-5 .io/proto/otlp/c/usr/bin/runc -o x_amd64/vet -W .cfg /tmp/go-build2862085962/b314/ x_amd64/cgo . --gdwarf2 --64 x_amd64/cgo(dns block)If you need me to access, download, or install something from one of these locations, you can either: