docs(maintenance): correct slog logging behavior after #218#222
Merged
veverkap merged 1 commit intoMay 10, 2026
Merged
Conversation
After commit 77541e8, StartCleanup resolves slog.Default() inline at each log site rather than capturing it once at startup. Update both docs/maintenance.md and README.md to reflect that the process-wide default logger is resolved at the time each log entry is written, so any slog.SetDefault call made after StartCleanup returns is immediately reflected in subsequent cleanup log entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates maintenance documentation to match the current maintenance.StartCleanup logging behavior after the change in #218, where the logger is resolved via slog.Default() at each log write (so late slog.SetDefault calls affect subsequent cleanup logs).
Changes:
- Correct the
maintenance.StartCleanuplogging bullet inREADME.md. - Apply the same correction in
docs/maintenance.md.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates the maintenance package section to describe per-log-entry slog.Default() resolution. |
| docs/maintenance.md | Updates the maintenance behavior documentation to match current slog.Default() call-site behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
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.
What
Commit 77541e8 (PR #218) changed
maintenance.StartCleanupto callslog.Default()inline at each log site rather than capturing the default logger once at the top ofrunCleaners. This eliminates a race where goroutine scheduling could cause the snapshot to precede a concurrentslog.SetDefaultcall.Both
docs/maintenance.mdandREADME.mdstill described the old behaviour:That statement is now incorrect — the logger is resolved at each log-write, not at startup.
Changes
docs/maintenance.md— update the logging bullet to match the actualslog.Default()call-site behaviour.README.md— apply the same correction to themaintenancepackage section.New wording
Testing
Documentation-only change; no code was modified. Verified against
maintenance/maintenance.golines 54–62.Warning
The following domain was blocked by the firewall during workflow execution:
proxy.golang.orgTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.
Greptile Summary
This PR corrects two documentation files that described the old (pre-#218) logging behaviour, where the
slog.Loggerwas captured once at startup. The source code now callsslog.Default()inline at each log site, and the docs are updated to match.docs/maintenance.mdandREADME.md: the logging bullet is reworded to state thatslog.Default()is resolved at each log-write, so any subsequentslog.SetDefaultcall is immediately reflected in cleanup log entries.maintenance.golines 54 and 62 exactly.Confidence Score: 5/5
Documentation-only change with no Go source modifications; safe to merge.
Both updated sentences match the actual slog.Default() call-site pattern in maintenance.go lines 54 and 62. The old wording was factually wrong and is now corrected in both locations. No code paths are touched.
No files require special attention.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Caller participant StartCleanup participant Goroutine participant slog Caller->>StartCleanup: StartCleanup(ctx, interval, cleaners...) StartCleanup->>Goroutine: launch background goroutine StartCleanup-->>Caller: return stop() Note over Caller: slog.SetDefault(newLogger) — reflected immediately Goroutine->>slog: slog.Default().ErrorContext(ctx, ...) [on error] slog-->>Goroutine: current default logger (newLogger) Goroutine->>slog: slog.Default().ErrorContext(ctx, ...) [on panic] slog-->>Goroutine: current default logger (newLogger) Caller->>StartCleanup: stop() StartCleanup-->>Caller: goroutine exitedReviews (1): Last reviewed commit: "docs(maintenance): correct slog logging ..." | Re-trigger Greptile