-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updating with fixes #258
Updating with fixes #258
Conversation
Signed-off-by: vgonuguntla <vinay_gonuguntla@intuit.com>
Signed-off-by: vgonuguntla <vinay_gonuguntla@intuit.com>
Signed-off-by: vgonuguntla <vinay_gonuguntla@intuit.com>
0cf2a1f
to
b826e36
Compare
admiral/pkg/clusters/types_test.go
Outdated
{ | ||
name: "Readonly test - Routing Policy", | ||
rp: &v1.RoutingPolicy{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: remove extra line.
admiral/pkg/clusters/types_test.go
Outdated
{ | ||
name: "Readonly false test - Routing Policy", | ||
rp: &v1.RoutingPolicy{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: remove extra line.
admiral/pkg/clusters/types_test.go
Outdated
t.Run(c.name, func(t *testing.T) { | ||
if c.readOnly { | ||
CurrentAdmiralState.ReadOnly = true | ||
}else{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: space before/after else.
e5cec4b
to
6fac595
Compare
@@ -184,3 +186,8 @@ func getHosts(routingPolicy *v1.RoutingPolicy) string { | |||
hosts = strings.TrimSuffix(hosts, ",") | |||
return hostsKey + hosts | |||
} | |||
|
|||
func getPlugin(routingPolicy *v1.RoutingPolicy) string { | |||
plugin := routingPolicy.Spec.Plugin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we check for nil here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops let me add that, its a check in quarter master so that should work
admiral/pkg/clusters/types_test.go
Outdated
doesError bool | ||
}{ | ||
{ | ||
name: "Readonly test - Routing Policy", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may be slightly more context here, for instance what we are trying to assert here.
Signed-off-by: vgonuguntla <vinay_gonuguntla@intuit.com>
6fac595
to
8e54dda
Compare
3 Changes -