younsl commented on code in PR #451: URL: https://github.com/apache/superset-kubernetes-operator/pull/451#discussion_r4300860883
########## docs/contributing/development-guidelines.md: ########## @@ -28,7 +28,7 @@ See [Architecture](../architecture/overview.md) for the structural overview (CRD - **Single public CRD**: `Superset` resolves shared spec (top-level + per-component) into parent-owned Kubernetes resources - **6 deployment components + lifecycle tasks**: web server, Celery worker, Celery beat, Flower, websocket, MCP, and task Jobs for seed/migrate/rotate/init -- **3 pure Go packages**: `internal/resolution/` (spec flattening), `internal/config/` (Python rendering), `internal/common/` (shared types) +- **4 pure Go packages**: `internal/resolution/` (spec flattening), `internal/config/` (Python rendering), `internal/common/` (shared types), `internal/schedule/` (cron tick computation) Review Comment: Good point. Dropped the count and kept a plain list of the current pure packages, so adding another one won't make the line wrong. ########## docs/contributing/development-guidelines.md: ########## @@ -299,7 +299,7 @@ New Deployment/Pod/Container fields go into the template hierarchy: 1. Determine the Kubernetes level: `DeploymentTemplate` (Deployment-level), `PodTemplate` (PodSpec-level), or `ContainerTemplate` (Container-level) 2. Add the field to the appropriate template type in `api/v1alpha1/shared_types.go` -3. Add the merge logic in `internal/resolution/merge.go` (`MergeDeploymentTemplate`, `MergePodTemplate`, or `MergeContainerTemplate`) using the field's natural semantics (scalar → `ResolveOverridableValue`, named slice → `Merge*ByName`, map → `MergeMaps`, unnamed slice → `append`). All merge functions follow the convention `Merge*(topLevel, component[, operatorInjected])`: the top-level value establishes ordering, the component value overrides by name in place, and where applicable (env vars, volume mounts, labels) operator-injected values are applied last so they cannot be overridden by user configuration. Any code consuming resolved ports, env vars, or other merged collections must respect this same ordering. +3. Add the merge logic in `internal/resolution/merge.go` (`MergeDeploymentTemplate`, `MergePodTemplate`, or `MergeContainerTemplate`) using the field's natural semantics (scalar → `ResolveOverridableValue`, named slice → the matching helper such as `MergeEnvVars` or `MergeVolumes`, map → `MergeMaps`, unnamed slice → `append`). The collection merge helpers take their inputs in the order `(topLevel, component[, operatorInjected])`: the top-level value establishes ordering, the component value overrides by name in place, and where applicable (env vars, volume mounts, labels) operator-injected values are applied last so they cannot be overridden by user configuration. Any code consuming resolved ports, env vars, or other merged collections must respect this same ordering. Review Comment: Agreed, and it turned out to be docs only, so I did it here. Step 3 now describes the merge rules in general terms (scalars overridden by the component value, named collections merged by name, maps by key, unnamed collections appended, applied in top-level, component, operator-injected order) without naming any function. ########## docs/contributing/development-guidelines.md: ########## @@ -340,7 +341,7 @@ internal/ | File | Purpose | |------|---------| | `api/v1alpha1/shared_types.go` | ImageSpec, MetastoreSpec, DeploymentTemplate, PodTemplate, ContainerTemplate, FlatComponentSpec | -| `api/v1alpha1/superset_types.go` | Parent SupersetSpec, component specs, InitSpec, CEL validation rules, status | +| `api/v1alpha1/superset_types.go` | Parent SupersetSpec, component specs, LifecycleSpec, CEL validation rules, status | Review Comment: Yep 😅 I also checked the rest of the Key Files table, and every path in it still exists. ########## docs/contributing/development-setup.md: ########## @@ -28,7 +28,7 @@ This guide walks through setting up a local development environment with Kind, P | Docker daemon | 20+ | [Docker Desktop](https://docs.docker.com/get-docker/), [Colima](https://github.com/abiosoft/colima) (`brew install colima`), or [Podman](https://podman.io) | | kubectl | 1.27+ | [kubernetes.io](https://kubernetes.io/docs/tasks/tools/) | | Kind | 0.30+ | `brew install kind` or `go install sigs.k8s.io/kind@latest` | -| Go | 1.26+ | `brew install go` or [go.dev](https://go.dev/dl/) | +| Go | 1.27+ | `brew install go` or [go.dev](https://go.dev/dl/) | Review Comment: It does. The docs now state the exact version (1.27.2), and a new regex manager in renovate.json tracks AGENTS.md and this file with the same golang Docker datasource as go.mod. They use the golang depName, so they join the existing Go toolchain group. I checked this with a local Renovate dry run: after setting go.mod, the Dockerfile, the devcontainer and both docs to 1.27.1, all five got the 1.27.2 patch on the same renovate/go-toolchain branch, and no digest was added to the docs. ########## AGENTS.md: ########## @@ -21,7 +21,7 @@ under the License. ## Project Overview -Kubernetes operator for Apache Superset, built with the Go-based Operator SDK and Go 1.26+. +Kubernetes operator for Apache Superset, built with the Go-based Operator SDK and Go 1.27+. Review Comment: Covered by the same Renovate manager, see the reply on development-setup.md. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
