Implement code review suggestion, round 2

projectsyn · Jul 25, 2024 · 9b99160 · 9b99160
1 parent 1165e59
commit 9b99160
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 9 deletions.
diff --git a/api/v1alpha1/cluster_types.go b/api/v1alpha1/cluster_types.go
@@ -134,7 +134,7 @@ func init() {
 // GetGitTemplate returns the git repository template
 func (c *Cluster) GetGitTemplate() *GitRepoTemplate {
 	if c.Spec.GitRepoTemplate == nil {
-		c.Spec.GitRepoTemplate = &GitRepoTemplate{}
+		return &GitRepoTemplate{}
 	}
 	return c.Spec.GitRepoTemplate
 }

diff --git a/api/v1alpha1/constants.go b/api/v1alpha1/constants.go
@@ -3,7 +3,7 @@ package v1alpha1
 const (
 	LabelNameTenant       = "syn.tools/tenant"
 	FinalizerName         = "cluster.lieutenant.syn.tools"
-	PipelineFinalizerName = "cluster.lieutenant.syn.tools"
+	PipelineFinalizerName = "cluster.lieutenant.syn.tools/pipelines"
 
 	// DeleteProtectionAnnotation defines the delete protection annotation name
 	DeleteProtectionAnnotation = "syn.tools/protected-delete"

diff --git a/controllers/cluster_compile_pipeline_controller.go b/controllers/cluster_compile_pipeline_controller.go
@@ -74,8 +74,7 @@ func (r *ClusterCompilePipelineReconciler) Reconcile(ctx context.Context, reques
 	// We can only get here if the cluster list and CI variables were successfully updated on the tenant.
 	// So in the case of deletion, we can clean up the finalizer here, because that update involves removing them.
 	if !instance.GetDeletionTimestamp().IsZero() {
-		if controllerutil.ContainsFinalizer(instance, synv1alpha1.PipelineFinalizerName) {
-			controllerutil.RemoveFinalizer(instance, synv1alpha1.PipelineFinalizerName)
+		if controllerutil.RemoveFinalizer(instance, synv1alpha1.PipelineFinalizerName) {
 			return ctrl.Result{}, r.Client.Update(ctx, instance)
 		}
 	}

diff --git a/controllers/util.go b/controllers/util.go
@@ -7,12 +7,12 @@ import (
 	v1 "k8s.io/api/core/v1"
 )
 
-func envVarIndex(name string, list *[]synv1alpha1.EnvVar) int {
-	return slices.IndexFunc(*list, func(e synv1alpha1.EnvVar) bool { return e.Name == name })
+func envVarIndex(name string, list []synv1alpha1.EnvVar) int {
+	return slices.IndexFunc(list, func(e synv1alpha1.EnvVar) bool { return e.Name == name })
 }
 
 func updateEnvVarValue(name string, value string, envVars []synv1alpha1.EnvVar) ([]synv1alpha1.EnvVar, bool) {
-	index := envVarIndex(name, &envVars)
+	index := envVarIndex(name, envVars)
 	changed := false
 	if index < 0 {
 		changed = true
@@ -30,7 +30,7 @@ func updateEnvVarValue(name string, value string, envVars []synv1alpha1.EnvVar)
 	return envVars, changed
 }
 func updateEnvVarValueFrom(name string, secret string, key string, protected bool, envVars []synv1alpha1.EnvVar) ([]synv1alpha1.EnvVar, bool) {
-	index := envVarIndex(name, &envVars)
+	index := envVarIndex(name, envVars)
 	changed := false
 	if index < 0 {
 		changed = true
@@ -65,7 +65,7 @@ func updateEnvVarValueFrom(name string, secret string, key string, protected boo
 }
 
 func removeEnvVar(name string, envVars []synv1alpha1.EnvVar) ([]synv1alpha1.EnvVar, bool) {
-	index := envVarIndex(name, &envVars)
+	index := envVarIndex(name, envVars)
 	if index >= 0 {
 		return slices.Delete(envVars, index, index+1), true