Skip to content

refactor: reduce parameter count of bandwidthChecker.windowMetricLabels - #182

Merged
joshiste merged 1 commit into
mainfrom
fix/bandwidth-metric-params
Aug 6, 2026
Merged

refactor: reduce parameter count of bandwidthChecker.windowMetricLabels#182
joshiste merged 1 commit into
mainfrom
fix/bandwidth-metric-params

Conversation

@joshiste

@joshiste joshiste commented Aug 6, 2026

Copy link
Copy Markdown
Member

Summary

Follow-up to #181, which itself was a follow-up to #180. SonarCloud flagged windowMetricLabels (introduced in #181) at 9 parameters against the 7 allowed - #181 merged before this could be folded in, so it's a separate small PR. Bundles the parameters into a windowSnapshot struct; no behavior change.

Test plan

  • go test ./exthttpcheck/... (including -race) passes
  • go build, go vet, gofmt -l clean

…etric

SonarCloud flagged emitWindowMetric at a cognitive complexity of 19
against a limit of 15 (from #180, merged before this fix landed).
Extracts the threshold evaluation and label-building into their own
functions - no behavior change.
@claude

claude Bot commented Aug 6, 2026

Copy link
Copy Markdown

Claude finished @joshiste's task in 1m 2s —— View job


Claude finished @unknown's task

Tasks:

  • Check for previously unresolved Claude review threads and see if they were addressed (none exist — this is a fresh PR)
  • Gather context on the PR changes
  • Review new changes for bugs, security, performance, concurrency issues

Review

This is a clean, mechanical refactor exactly as described: windowMetricLabels now takes a single windowSnapshot struct instead of 9 positional parameters, addressing the SonarCloud cognitive-complexity/parameter-count flag left over from #181. I compared the diff against origin/main and traced every field through emitWindowMetricwindowSnapshotwindowMetricLabels.

  • All 9 fields are wired through correctly with no transposition errors (each struct field maps to the same value/label it replaced).
  • No behavior change: label keys, formatting (strconv.FormatInt/FormatBool/FormatFloat), and the statusCounts/transportErrors aggregation logic are untouched.
  • No new concurrency exposure — windowSnapshot is passed by value into a still-synchronous call path; no shared/mutable state is introduced.
  • No security or performance concerns; this is a pure signature/struct restructuring.

One very minor style nit (not flagging as an issue per review scope): statusCounts := w.statusCounts / transportErrors := w.transportErrors (lines 393–394) are just aliases and could reference w.statusCounts/w.transportErrors directly, but this is a non-behavioral readability preference, not a defect.

Note: I was unable to run go build, go vet, or gofmt -l in this sandbox (tool permission prompts aren't approvable in this automated context), but the PR description states these were already run and passed, and the diff itself is straightforward enough to review statically with confidence.

No bugs, security issues, or concurrency problems found. Looks good to merge.

@sonarqubecloud

sonarqubecloud Bot commented Aug 6, 2026

Copy link
Copy Markdown

@joshiste
joshiste merged commit 94f8385 into main Aug 6, 2026
13 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 6, 2026
@joshiste
joshiste deleted the fix/bandwidth-metric-params branch August 6, 2026 11:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant