refactor: consolidate reinvented semver helpers into pkg/semverutil - #42156
Conversation
- Add IsMorePreciseVersion to pkg/semverutil - Replace ensureSemverPrefix/semverMajorMinorParts in compile_update_check.go with semverutil.EnsureVPrefix/semverutil.ParseVersion - Replace ensureSemverPrefix in update_check.go with semverutil.EnsureVPrefix - Remove isMorePreciseVersion from action_cache.go, use semverutil.IsMorePreciseVersion - Update action_cache_test.go to call semverutil.IsMorePreciseVersion Closes #42151 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Remove redundant nil checks after ParseVersion (already guarded by IsValid) - Clarify IsMorePreciseVersion doc comment re: no input validation Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Hey One thing to tidy up before this is ready for review:
If you'd like a hand polishing this up, you can use the following prompt:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors semver-related logic to remove duplicated helpers in feature code and route callers through the shared pkg/semverutil package, improving consistency and reducing drift across the CLI and workflow packages.
Changes:
- Added
semverutil.IsMorePreciseVersionand migratedpkg/workflow/action_cache.go+ tests to use it. - Replaced local semver helpers and direct
golang.org/x/mod/semverusage in CLI update-check code withsemverutilwrappers. - Simplified CLI semver parsing logic by using
semverutil.ParseVersionfor major/minor comparisons.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/action_cache.go | Switched dedup sort logic to use semverutil.IsMorePreciseVersion and removed the local helper. |
| pkg/workflow/action_cache_test.go | Updated precision-comparison tests to call into semverutil. |
| pkg/semverutil/semverutil.go | Added exported IsMorePreciseVersion helper for shared use. |
| pkg/cli/update_check.go | Replaced x/mod/semver calls with semverutil wrappers for validation and comparison. |
| pkg/cli/compile_update_check.go | Replaced local semver prefix/major-minor parsing helpers with semverutil functions. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Low
| currentParsed := semverutil.ParseVersion(currentSV) | ||
| latestParsed := semverutil.ParseVersion(latestSV) | ||
|
|
||
| major, err := strconv.Atoi(parts[0]) | ||
| if err != nil { | ||
| return 0, 0, false | ||
| } | ||
| minor, err := strconv.Atoi(parts[1]) | ||
| if err != nil { | ||
| return 0, 0, false | ||
| } | ||
|
|
||
| return major, minor, true | ||
| } | ||
|
|
||
| func ensureSemverPrefix(version string) string { | ||
| if strings.HasPrefix(version, "v") { | ||
| return version | ||
| } | ||
| return "v" + version | ||
| return currentParsed.Major == latestParsed.Major && latestParsed.Minor > currentParsed.Minor |
| // IsMorePreciseVersion reports whether v1 is more version-precise than v2. | ||
| // Precision is measured by the number of dot-separated components: "v4.3.0" | ||
| // (three components) is more precise than "v4" (one component). When both | ||
| // versions have the same number of components, lexicographic ordering is used. | ||
| // No validation is performed; the function counts dots in the raw strings, so | ||
| // callers should ensure both inputs are well-formed version tags. |
|
@copilot Please address the review feedback, refresh this branch from main, and run the pr-finisher skill to bring the PR to ready state.
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed the requested review feedback in
Branch is already refreshed from |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in I pulled the failing job logs, reproduced the failures locally, and updated the wasm golden snapshots to match the current awf/firewall version ( Validation run:
|
|
🎉 This pull request is included in a new release. Release: |
Three semver helpers had been independently reimplemented in feature files despite
pkg/semverutilalready providing equivalent functionality. This PR removes the duplicates and routes all callers through the shared package.Changes
pkg/semverutil— addsIsMorePreciseVersion(v1, v2 string) bool, the canonical replacement for the privateisMorePreciseVersioninaction_cache.gopkg/cli/compile_update_check.go— removesensureSemverPrefix(≡semverutil.EnsureVPrefix) andsemverMajorMinorParts(≡semverutil.ParseVersion.Major/.Minor); replaces directgolang.org/x/mod/semvercalls withsemverutilwrappers; dropsstrconvimportpkg/cli/update_check.go— sameensureSemverPrefix/semver.*replacementpkg/workflow/action_cache.go— removesisMorePreciseVersion, callssemverutil.IsMorePreciseVersionpkg/workflow/action_cache_test.go— updates test to callsemverutil.IsMorePreciseVersionBefore:
After: