You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by "younsl (via GitHub)" <gi...@apache.org> on 2026/09/24 06:52:24 UTC

[PR] refactor: apply go fix modernizers across codebase [superset-kubernetes-operator]

younsl opened a new pull request, #388:
URL: https://github.com/apache/superset-kubernetes-operator/pull/388

   ## Summary
   
   Apply [go fix](https://pkg.go.dev/cmd/fix) across the whole module so the code uses the idioms available at the Go version declared in go.mod (1.27). Since Go 1.26, go fix runs the [modernize](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize) and [inline](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/inline) analyzers, and a fresh checkout of main currently produces a large pending diff. Landing it once in a purely mechanical PR keeps future feature PRs free of unrelated modernization noise. [gofmt](https://pkg.go.dev/cmd/gofmt) with simplification was also run and reports no changes, so the tree was already formatted. There is no behavior change.
   
   ## Details
   
   - Pointer helpers: common.Ptr(x) and the local helpers (boolPtr, int32Ptr, ptrInt32, pathTypePtr, the ptr test helper, resolution.Ptr) are replaced with the [new(expr)](https://go.dev/doc/go1.26) form introduced in Go 1.26. common.Ptr is kept and annotated with a go:fix inline directive, so any future caller is migrated automatically by go fix.
   - Promoted fields in composite literals: 176 nested ObjectMeta and TypeMeta literals now set Name, Namespace, and similar fields directly on the outer object, as allowed from [Go 1.27](https://go.dev/doc/go1.27). The metav1 imports that became unused are dropped.
   - Smaller modernizations: interface{} becomes any, a manual map copy becomes [maps.Copy](https://pkg.go.dev/maps#Copy), a clamp becomes the [max](https://go.dev/ref/spec#Min_and_max) builtin, and a split loop in test/utils uses [strings.SplitSeq](https://pkg.go.dev/strings#SplitSeq).
   - go fix was run with the default and integration build tags and repeated until it produced no further diff.
   - Manual follow up, limited to finishing what the inliner started: it cannot inline calls whose argument is an untyped constant (for example common.EnvironmentDev) because that needs type parameter inference, which govet reports as an error. Those calls, plus the resolution.Ptr calls in tests, were converted to new(expr) by hand, and the helpers left without callers were deleted to satisfy the unused linter.
   - 57 files changed, all under internal/ and test/utils. No API types, CRDs, generated code, or chart files are touched.
   
   <details>
   <summary>Verification</summary>
   
   - go build and go vet pass for both the default and integration build tags.
   - go fix reports an empty diff for both tag sets after this change.
   - gofmt with simplification lists no files.
   - make lint-go ([golangci-lint](https://golangci-lint.run/) fmt and run) reports 0 issues.
   - make test-unit passes for every package (controller 91.5%, config 99.5%, resolution 99.2% statement coverage).
   - make test-integration passes with [envtest](https://book.kubebuilder.io/reference/envtest), including the CEL validation and controller suites.
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor: apply go fix modernizers across codebase [superset-kubernetes-operator]

Posted by "younsl (via GitHub)" <gi...@apache.org>.

younsl commented on PR #388:
URL: https://github.com/apache/superset-kubernetes-operator/pull/388#issuecomment-5825084980

@villebro Thanks for the review! Both points are addressed in b1f8b9e.

1. Removed common.Ptr along with its test, which after go fix only exercised the new builtin. The contributing guide example and AGENTS.md no longer mention the Ptr helper.
2. modernize is now enabled in .golangci.yml, so make lint-go and CI fail on new drift. For the integration tag I used run.build-tags in .golangci.yml instead of a flag on the lint-go target, so it also applies to lint-go-fix and editor integrations. Turning it on surfaced one more dead helper in the envtest suite (strPtr), now removed.

To confirm the rule fires, I linted a throwaway probe file (not committed) containing interface{}, a nested ObjectMeta literal, an index loop, and a generic pointer helper. modernize flagged all four (any, embedlit, rangeint, newexpr). go build, go vet, go fix (no remaining diff), make lint-go, make test-unit, make test-integration, make codegen, and make lint-md all pass, with both the default and integration tags where applicable.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor: apply go fix modernizers across codebase [superset-kubernetes-operator]

Posted by "villebro (via GitHub)" <gi...@apache.org>.

villebro commented on code in PR #388:
URL: https://github.com/apache/superset-kubernetes-operator/pull/388#discussion_r4097455529


##########
internal/common/types.go:
##########
@@ -42,6 +42,8 @@ const (
 )
 
 // Ptr returns a pointer to the given value.
+//
+//go:fix inline
 func Ptr[T any](v T) *T {
-	return &v
+	return new(v)

Review Comment:
   Let's just delete this



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor: apply go fix modernizers across codebase [superset-kubernetes-operator]

Posted by "younsl (via GitHub)" <gi...@apache.org>.

younsl commented on code in PR #388:
URL: https://github.com/apache/superset-kubernetes-operator/pull/388#discussion_r4100343233


##########
internal/common/types.go:
##########
@@ -42,6 +42,8 @@ const (
 )
 
 // Ptr returns a pointer to the given value.
+//
+//go:fix inline
 func Ptr[T any](v T) *T {
-	return &v
+	return new(v)

Review Comment:
   Done in b1f8b9e.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor: apply go fix modernizers across codebase [superset-kubernetes-operator]

Posted by "villebro (via GitHub)" <gi...@apache.org>.

villebro merged PR #388:
URL: https://github.com/apache/superset-kubernetes-operator/pull/388


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] refactor: apply go fix modernizers across codebase [superset-kubernetes-operator]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.

codecov-commenter commented on PR #388:
URL: https://github.com/apache/superset-kubernetes-operator/pull/388#issuecomment-5820354225

   ## [Codecov](https://app.codecov.io/gh/apache/superset-kubernetes-operator/pull/388?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `98.73418%` with `1 line` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 93.12%. Comparing base ([`d3ade92`](https://app.codecov.io/gh/apache/superset-kubernetes-operator/commit/d3ade92665e9144697a0d90876f135c68dfbad43?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`77a3d31`](https://app.codecov.io/gh/apache/superset-kubernetes-operator/commit/77a3d31721cd8ee373fda6a8374e63bc1f0405b4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/superset-kubernetes-operator/pull/388?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [internal/common/types.go](https://app.codecov.io/gh/apache/superset-kubernetes-operator/pull/388?src=pr&el=tree&filepath=internal%2Fcommon%2Ftypes.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aW50ZXJuYWwvY29tbW9uL3R5cGVzLmdv) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/superset-kubernetes-operator/pull/388?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset-kubernetes-operator/pull/388?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org