Conversation
…Pin helper - Replace regexp.MatchString (which recompiles on every call) with a package-level precompiled fullSHARegex variable for better performance - Extract findCompatiblePin as a pure, named helper function, replacing the var/bool mutation pattern used to find the first semver-compatible pin This makes the logic easier to read and test in isolation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors pkg/actionpins to reduce per-call overhead in SHA validation and to simplify compatible-pin selection logic.
Changes:
- Precompiles the full SHA validation regex at package scope and reuses it in
isValidFullSHA. - Extracts compatible-pin selection into a small helper (
findCompatiblePin) and removes the mutable flag pattern at the call site.
Show a summary per file
| File | Description |
|---|---|
| pkg/actionpins/actionpins.go | Precompiles SHA regex and extracts findCompatiblePin helper for cleaner pin selection. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 1
| @@ -196,8 +199,18 @@ func ExtractVersion(uses string) string { | |||
|
|
|||
| // isValidFullSHA checks if a string is a valid 40-character hexadecimal SHA. | |||
There was a problem hiding this comment.
The isValidFullSHA doc comment says “hexadecimal SHA”, but the regex only accepts lowercase ([0-9a-f]). Consider clarifying the comment to explicitly say “lowercase hex” (or broaden the regex to accept uppercase too) to avoid ambiguity for callers.
Suggested change
| // isValidFullSHA checks if a string is a valid 40-character hexadecimal SHA. | |
| // isValidFullSHA checks if a string is a valid 40-character lowercase hexadecimal SHA. |
Closed
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.
Round-Robin Progress
Package processed:
pkg/actionpins(1st package — first run, no previous cache)Next run will process:
pkg/agentdrainSummary
Two targeted functional/immutability improvements applied to
pkg/actionpins/actionpins.go:regexp.MustCompileto package levelactionpins.gofindCompatiblePinhelper, eliminate mutable flagactionpins.goChanges
1. Precompiled
fullSHARegexisValidFullSHApreviously calledregexp.MatchString(...)on every invocation, which recompiles the pattern each time. The pattern is now a package-levelvarcompiled once at startup:Benefits: better performance (no repeated compilation), cleaner code, eliminates the ignored
errreturn.2. Extracted
findCompatiblePinpure helperThe compatible-pin search loop used a mutable
foundCompatible boolflag and a pre-declaredvar selectedPin ActionPin. This has been extracted into a named pure function:Benefits: eliminates mutable intermediate state, gives the operation a clear name, isolates the logic for independent testability.
Testing
go build ./pkg/actionpins/— cleango vet ./pkg/actionpins/— no issuesTestActionPin*andTestGetActionPin*tests inpkg/workflow/pass — no behavior changeReview Focus
fullSHARegexplacement and naming is idiomaticfindCompatiblePinsignature and doc comment are sufficient