Skip to content

refactor(gthulhu): copy the applied set with maps.Clone and tidy comments - #18

Open
thc1006 wants to merge 1 commit into
Gthulhu:mainfrom
thc1006:refactor/gthulhu-modernize-comments
Open

thc1006 wants to merge 1 commit into
Gthulhu:mainfrom
thc1006:refactor/gthulhu-modernize-comments

Conversation

@thc1006

@thc1006 thc1006 commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

What does this PR change?

Follow-up to #17, no behavior change. In GetChangedStrategies the applied set is rebuilt at the end of each drain. It used a manual make plus a copy loop, now it is maps.Clone(g.strategyMap) which does the same thing. The concurrency test uses for range instead of for i := 0; i < n; i++, and I shortened the comment on the strategyMap / appliedStrategyMap fields so it is not longer than the two fields it describes.

Why is this change needed?

Same reason as the api side follow-up: after #17 merged I read my own code again against modern Go and the comment style this repo prefers. The module targets Go 1.22, so maps.Clone is available and reads better than the hand written copy. The coalescing logic is untouched.

Follow-up to #17.

How was it tested?

On Linux:

  • gofmt -s -l on the two files is clean
  • go vet ./plugin/gthulhu/... passes
  • go test -race ./plugin/gthulhu/... passes, so the concurrency test still guards the write lock under the race detector

Contributor checklist

  • The change is focused and does not include unrelated cleanup.
  • Tests were added or updated when behavior changed. (no behavior change, existing tests kept)
  • Documentation was updated when user-facing behavior changed. (N/A)
  • Relevant local/CI checks pass.
  • No secrets or sensitive environment data are included.

Scheduling / eBPF changes

N/A. User space plugin code only.

…ents

Follow-up polish to the merged Gthulhu#17. No behavior change.

Replace the hand-written applied-set copy in GetChangedStrategies with maps.Clone, use for range in the concurrency test, and shorten the strategyMap/appliedStrategyMap field comment so it is not longer than the two fields it documents.

gofmt -s, go vet and go test -race are clean.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant