chore: modernize using slices, min builtin, and %w error wrapping#2916
chore: modernize using slices, min builtin, and %w error wrapping#2916mrueg wants to merge 1 commit intokubernetes:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mrueg The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
|
k8s-ci-robot
added
the
approved
|
This issue is currently awaiting triage. If kube-state-metrics contributors determine this is a relevant issue, they will accept it by applying the The DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
the
needs-triage
k8s-ci-robot
added
size/M
mrueg
changed the title
mrueg
force-pushed
the
modernize-go-primitives
branch
from
5baa5f1 to
b01a72c
Compare
k8s-ci-robot
added
cncf-cla: yes
There was a problem hiding this comment.
Pull request overview
Modernizes the Go codebase by adopting newer standard-library helpers for sorting, min selection, and error wrapping to improve determinism and error-chain preservation.
Changes:
- Replace
sort.*usage withslices.Sort/slices.SortFuncfor deterministic ordering. - Replace
math.Min(float64(len(...)))with themin()builtin. - Wrap returned errors with
%win several paths to preserveerrors.Is/Asbehavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/options/types.go | Switches set stringification sorting to slices.Sort. |
| pkg/optin/optin.go | Uses slices.Sort to keep opt-in status output deterministic. |
| pkg/metrics_store/metrics_writer.go | Wraps write errors with %w for proper error chaining. |
| pkg/customresourcestate/registry_factory.go | Modernizes sorting and min usage; wraps certain errors with %w. |
| pkg/customresourcestate/registry_factory_test.go | Updates tests to compare error messages rather than concrete error types. |
| pkg/app/server.go | Converts multiple error returns to %w wrapping. |
| internal/store/utils.go | Updates label-key sorting to slices.Sort. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (c *compiledGauge) Values(v interface{}) (result []eachValue, errs []error) { | ||
| onError := func(err error) { | ||
| errs = append(errs, fmt.Errorf("%s: %v", c.Path(), err)) | ||
| errs = append(errs, fmt.Errorf("%s: %w", c.Path(), err)) | ||
| } |
There was a problem hiding this comment.
PR description mentions switching fmt.Errorf("%v", err) to "%w" to preserve error chains. This file updates compiledGauge.Values to wrap with %w, but compiledInfo.Values still formats aggregated errors with %v (and therefore doesn’t preserve unwrap-able causes). Either update compiledInfo.Values similarly (e.g., by joining/wrapping underlying errors) or adjust the PR description/scope so it doesn’t imply full conversion.
k8s-ci-robot
added
cncf-cla: no
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Replace sort.StringSlice + .Sort() with []string + slices.Sort()
- Replace sort.Slice with slices.SortFunc (typed comparator)
- Replace sort.Strings with slices.Sort across multiple packages
- Replace math.Min(float64(len(...))) with min() builtin (Go 1.21)
- Replace fmt.Errorf("%v", err) with "%w" to preserve error chains
- Update test to compare error messages rather than concrete error types
mrueg
force-pushed
the
modernize-go-primitives
branch
from
a47febd to
90d1ad7
Compare
k8s-ci-robot
added
cncf-cla: yes
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
What this PR does / why we need it:
How does this change affect the cardinality of KSM: (increases, decreases or does not change cardinality)
Which issue(s) this PR fixes: (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged)Fixes #