{{ message }}
refactor: Semantic function clustering — split strutil.go, extract githubhttp package, fix validation.go placement, rename logger/common.go#8244
Merged
Conversation
…provements Addresses issue #8207 with four refactoring improvements: 1. strutil: Split catch-all strutil.go into focused files matching test files - deepclone.go: DeepCloneJSON (with improved docs) - util.go: SortedSetKeys, GetStringFromMap - deduplicate.go: DeduplicateStrings - strings_to_any.go: StringsToAny - copy_trimmed_string_int_map.go: CopyTrimmedStringIntMap - Remove empty strutil.go 2. httputil -> githubhttp: Extract GitHub API-specific code into dedicated package - githubhttp/client.go: ApplyGitHubAPIHeaders, DoGitHubGET, ParseRateLimitResetHeader, ComputeRetryAfter - githubhttp/collaborator.go: ParseCollaboratorPermissionArgs, LogAndWrapCollaboratorPermission, FetchCollaboratorPermission - Update server/unified.go, proxy/proxy.go, proxy/handler.go callers - Move tests to githubhttp package 3. config: Move validateRuleBasedPatterns from validation_rules.go (atomic rules) to validation.go (composite orchestrators) 4. logger: Rename common.go -> global_state.go to better communicate purpose
Copilot
AI
changed the title
[WIP] Refactor semantic function clustering for organization improvements
refactor: Semantic function clustering — split strutil.go, extract githubhttp package, fix validation.go placement, rename logger/common.go
Jun 28, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors internal packages and file layout to better align source files with their responsibilities (string utilities, GitHub-HTTP helpers, config validation orchestration, and logger global state), improving navigability and reducing mixed concerns.
Changes:
- Split
internal/strutil/strutil.gointo focused files (util.go,deduplicate.go,deepclone.go, etc.) and removed the catch-all file. - Extract GitHub API–specific HTTP helpers into a new
internal/githubhttppackage and update server/proxy callers + tests accordingly. - Move
validateRuleBasedPatternsintointernal/config/validation.goand rename logger “common” file toglobal_state.gowith matching test rename.
Show a summary per file
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
internal/githubhttp/client.go:28
- The ApplyGitHubAPIHeaders doc comment uses "******" as an Authorization header example, which is misleading and obscures the expected scheme (token vs Bearer). Use a placeholder that preserves the header format (e.g., "Bearer ").
- Files reviewed: 19/21 changed files
- Comments generated: 0
- Review effort level: Low
This was referenced Jun 28, 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.
Four file-organization issues identified by semantic function clustering analysis where source files contradicted naming conventions or mixed concerns.
1.
strutil/strutil.go→ dedicated filesSplit the catch-all into files that match the existing
*_test.goconventions:deepclone.goDeepCloneJSONutil.goSortedSetKeys,GetStringFromMapdeduplicate.goDeduplicateStringsstrings_to_any.goStringsToAnycopy_trimmed_string_int_map.goCopyTrimmedStringIntMapstrutil.goremoved (no remaining content).2.
httputil→internal/githubhttp/(new package)GitHub API–specific code was co-located with generic HTTP utilities. Extracted into a dedicated package:
Updated callers:
server/unified.go,proxy/proxy.go,proxy/handler.go,proxy/rate_limit_test.go. Tests moved togithubhttp/.3.
config/validation_rules.go→validation.govalidateRuleBasedPatternsis a composite orchestrator (iterates all servers, callsvalidateGatewayConfig), not an atomic rule. Moved tovalidation.gowhere all other orchestrators live.4.
logger/common.go→logger/global_state.gocommonis a Go anti-pattern filename. The file manages global logger state with mutex locking —global_state.gocommunicates this. Test file renamed to match.