Refactor parameter names for clarity

internal/controller/conditionsutil/conditions_util.go

@@ -29,17 +29,17 @@ const (
 // since Pinniped should always check every condition.
 // It returns true if any resulting condition has non-true status.
 func MergeConditions(
-	conditions []*metav1.Condition,
-	conditionsToUpdate *[]metav1.Condition,
+	newConditions []*metav1.Condition,
+	existingConditionsToUpdate *[]metav1.Condition,
 	observedGeneration int64,
 	lastTransitionTime metav1.Time,
 	log plog.MinLogger,
 ) bool {
-	for i := range conditions {
-		cond := conditions[i].DeepCopy()
+	for i := range newConditions {
+		cond := newConditions[i].DeepCopy()
 		cond.LastTransitionTime = lastTransitionTime
 		cond.ObservedGeneration = observedGeneration
-		if mergeCondition(conditionsToUpdate, cond) {
+		if mergeCondition(existingConditionsToUpdate, cond) {
 			log.Info("updated condition",
 				"type", cond.Type,
 				"status", cond.Status,
@@ -47,39 +47,39 @@ func MergeConditions(
 				"message", cond.Message)
 		}
 	}
-	sort.SliceStable(*conditionsToUpdate, func(i, j int) bool {
-		return (*conditionsToUpdate)[i].Type < (*conditionsToUpdate)[j].Type
+	sort.SliceStable(*existingConditionsToUpdate, func(i, j int) bool {
+		return (*existingConditionsToUpdate)[i].Type < (*existingConditionsToUpdate)[j].Type
 	})
-	return HadErrorCondition(conditions)
+	return HadErrorCondition(newConditions)
 }
 
 // mergeCondition merges a new metav1.Condition into a slice of existing conditions. It returns true
-// if the condition has meaningfully changed.
-func mergeCondition(existing *[]metav1.Condition, new *metav1.Condition) bool {
+// if something other than the LastTransitionTime has been updated.
+func mergeCondition(existingConditionsToUpdate *[]metav1.Condition, newCondition *metav1.Condition) bool {
 	// Find any existing condition with a matching type.
 	var old *metav1.Condition
-	for i := range *existing {
-		if (*existing)[i].Type == new.Type {
-			old = &(*existing)[i]
+	for i := range *existingConditionsToUpdate {
+		if (*existingConditionsToUpdate)[i].Type == newCondition.Type {
+			old = &(*existingConditionsToUpdate)[i]
 			continue
 		}
 	}
 
 	// If there is no existing condition of this type, append this one and we're done.
 	if old == nil {
-		*existing = append(*existing, *new)
+		*existingConditionsToUpdate = append(*existingConditionsToUpdate, *newCondition)
 		return true
 	}
 
 	// Set the LastTransitionTime depending on whether the status has changed.
-	new = new.DeepCopy()
-	if old.Status == new.Status {
-		new.LastTransitionTime = old.LastTransitionTime
+	newCondition = newCondition.DeepCopy()
+	if old.Status == newCondition.Status {
+		newCondition.LastTransitionTime = old.LastTransitionTime
 	}
 
 	// If anything has actually changed, update the entry and return true.
-	if !equality.Semantic.DeepEqual(old, new) {
-		*old = *new
+	if !equality.Semantic.DeepEqual(old, newCondition) {
+		*old = *newCondition
 		return true
 	}
 

internal/controller/conditionsutil/conditions_util_test.go

@@ -21,13 +21,13 @@ func TestMergeIDPConditions(t *testing.T) {
 	testTime := metav1.Now()
 
 	tests := []struct {
-		name               string
-		newConditions      []*metav1.Condition
-		conditionsToUpdate *[]metav1.Condition
-		observedGeneration int64
-		wantResult         bool
-		wantLogSnippets    []string
-		wantConditions     []metav1.Condition
+		name                       string
+		newConditions              []*metav1.Condition
+		existingConditionsToUpdate *[]metav1.Condition
+		observedGeneration         int64
+		wantResult                 bool
+		wantLogSnippets            []string
+		wantConditions             []metav1.Condition
 	}{
 		{
 			name: "Adding a new condition with status=True returns false",
@@ -39,8 +39,8 @@ func TestMergeIDPConditions(t *testing.T) {
 					Message: "new message",
 				},
 			},
-			observedGeneration: int64(999),
-			conditionsToUpdate: &[]metav1.Condition{},
+			observedGeneration:         int64(999),
+			existingConditionsToUpdate: &[]metav1.Condition{},
 			wantLogSnippets: []string{
 				`"message":"updated condition","type":"NewType","status":"True"`,
 			},
@@ -78,7 +78,7 @@ func TestMergeIDPConditions(t *testing.T) {
 					Message: "new message",
 				},
 			},
-			conditionsToUpdate: &[]metav1.Condition{
+			existingConditionsToUpdate: &[]metav1.Condition{
 				{
 					Type:               "UnchangedType",
 					Status:             metav1.ConditionTrue,
@@ -140,7 +140,7 @@ func TestMergeIDPConditions(t *testing.T) {
 					Message: "unchanged message",
 				},
 			},
-			conditionsToUpdate: &[]metav1.Condition{
+			existingConditionsToUpdate: &[]metav1.Condition{
 				{
 					Type:               "UnchangedType",
 					Status:             metav1.ConditionFalse,
@@ -165,26 +165,26 @@ func TestMergeIDPConditions(t *testing.T) {
 		},
 	}
 
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
 			var log bytes.Buffer
 			logger := plog.TestLogger(t, &log)
 
 			result := MergeConditions(
-				tt.newConditions,
-				tt.conditionsToUpdate,
-				tt.observedGeneration,
+				test.newConditions,
+				test.existingConditionsToUpdate,
+				test.observedGeneration,
 				testTime,
 				logger,
 			)
 
 			logString := log.String()
-			require.Equal(t, len(tt.wantLogSnippets), strings.Count(logString, "\n"))
-			for _, wantLog := range tt.wantLogSnippets {
+			require.Equal(t, len(test.wantLogSnippets), strings.Count(logString, "\n"))
+			for _, wantLog := range test.wantLogSnippets {
 				require.Contains(t, logString, wantLog)
 			}
-			require.Equal(t, tt.wantResult, result)
-			require.Equal(t, tt.wantConditions, *tt.conditionsToUpdate)
+			require.Equal(t, test.wantResult, result)
+			require.Equal(t, test.wantConditions, *test.existingConditionsToUpdate)
 		})
 	}
 }