Conversation
Co-authored-by: Björn Kottner <BjoernKarma@users.noreply.github.com> Co-authored-by: Ismael Garba <iagarba@users.noreply.github.com> Co-authored-by: Stefan Siber <stefan-ctrl@users.noreply.github.com> Co-authored-by: Ron Gummich <ron96g@users.noreply.github.com>
Co-authored-by: Björn Kottner <BjoernKarma@users.noreply.github.com> Co-authored-by: Ismael Garba <iagarba@users.noreply.github.com> Co-authored-by: Stefan Siber <stefan-ctrl@users.noreply.github.com> Co-authored-by: Ron Gummich <ron96g@users.noreply.github.com>
| Eventually(func() bool { | ||
| err := k8sClient.Get(ctx, zone1Ref, zone1) | ||
| return err == nil && meta.IsStatusConditionTrue(zone1.Status.Conditions, condition.ConditionTypeReady) | ||
| }, timeout, interval).Should(BeTrue()) |
There was a problem hiding this comment.
optional: better way is use With functions. It is adding directly additional context because without knowing Eventually interface you can very easy switch timeout and interval variable or another stuff because it is very generic
|
|
||
| // Build URLs array: all DTC URLs from all zones (including current zone) | ||
| var dtcUrls []string | ||
| for _, zone := range zoneList.Items { |
There was a problem hiding this comment.
how many zones can we have here? for _, zone := range will copy whole object in .Items. Use index can avoid necessary operation and improve performance.
If operation is executed on stack, it won't bring a lot of improvement.
| url, err := url.Parse(r.Spec.Url) | ||
| // Use the first URL as the upstream URL | ||
| if len(r.Spec.Urls) == 0 { | ||
| return ups, err |
There was a problem hiding this comment.
optional: return here is enough but err will be nill and ups will be empty. Is it okay?
| } | ||
|
|
||
| _, err = c.CreateOrUpdate(ctx, route, mutator) | ||
| _, err := c.CreateOrUpdate(ctx, route, mutator) |
| mutator := func() error { | ||
| err := controllerutil.SetControllerReference(realm, route, c.Scheme()) | ||
| if err != nil { | ||
| if err := controllerutil.SetControllerReference(realm, route, c.Scheme()); err != nil { |
There was a problem hiding this comment.
optional: i don't recommend use the same variable name in nested function because it can be very easy be included over closures and it complicated review because difference is almost just one character := vs =. I prefer use custom names as locErr (local Error) or something else.
2 participants
No description provided.