fix: resolve calendar field shadowing, Retry-After header, log ordering, CHANGELOG - #36
Closed
calmcacil wants to merge 1 commit into
Closed
fix: resolve calendar field shadowing, Retry-After header, log ordering, CHANGELOG#36calmcacil wants to merge 1 commit into
calmcacil wants to merge 1 commit into
Conversation
…ng, CHANGELOG Fixes from second architecture review: - Remove shadowing WatchMode/WatchSeconds fields from calendar.ToolConfig (the outer fields silently shadowed embedded core.CommonConfig.Watch and core.CommonConfig.WatchSeconds; renamed cfg.WatchMode -> cfg.Watch) - Add HTTP Retry-After header support to DoRequest (was only parsing JSON body for retryAfter, ignoring the standard header; now prefers the header and falls back to body parsing) - Replace slogx.Default().Warn() with fmt.Fprintf in LoadAndValidate (startup warming was emitted before logger was configured, causing text-only format even under --json) - Fix CHANGELOG incorrectly claiming NewTransportClient was removed
Owner
Author
|
Superseded by the unified CLI modernization in #37; closing before the public-history sanitization. |
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
1 participant
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.
Changes
Four fixes from the second architecture review:
1. Calendar field shadowing
Removed redundant
WatchMode/WatchSecondsfields fromcalendar.ToolConfigthat silently shadowed the embeddedcore.CommonConfig.Watch/core.CommonConfig.WatchSeconds. Renamedcfg.WatchMode→cfg.Watch(3 occurrences inrender.go, 1 inmain.go). Now the embedding is real instead of half-broken.2. HTTP Retry-After header support
parseRetryAfterwas only parsing the JSON response body for aretryAfterfield. AddedparseRetryAfterHeaderthat reads the standard HTTPRetry-Afterheader (RFC 7231 §7.1.3). Sonarr/Radarr return this as a header. ModifiedDoRequestto extract it from the response and return alongside body/status.doWithRetryprefers the header value and falls back to body parsing.3. LoadAndValidate log ordering
Replaced
slogx.Default().Warn(...)withfmt.Fprintf(os.Stderr, ...)inLoadAndValidate. Startup diagnostics were emitted before the slog logger was configured (sinceLoadAndValidateruns beforeflag.Parse/Apply), causing text-only output even under--json.4. CHANGELOG doc fix
Removed the incorrect line claiming
NewTransportClient (dead code)was removed.NewTransportClientis still used bycalendar.goandairtime.go.fix/arch-review-findingsmaingofmt,go vet,go build,go test -race: all clean