Replaced use of strings.ToLower comparison with EqualFold #214
Conversation
|
This change is |
|
oh, my regex wasn't the best, 1 sec |
tothandras
changed the title
tothandras
changed the title
Codecov Report
@@ Coverage Diff @@
## master #214 +/- ##
======================================
Coverage 96.7% 96.7%
======================================
Files 32 32
Lines 1639 1639
======================================
Hits 1585 1585
Misses 41 41
Partials 13 13
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Looks really good!
There's one change that shouldn't be there, for a source file within the vendor directory. Apart from that, there are a couple of other places missing:
./pkg/deployment/agent.go: if strings.ToLower(a.jaeger.Spec.Agent.Strategy) != "daemonset" {
./pkg/cronjob/spark_dependencies.go: return supportedStorageTypes[strings.ToLower(storage)]
./pkg/strategy/controller.go: if strings.ToLower(jaeger.Spec.Strategy) != "production" && strings.ToLower(jaeger.Spec.Strategy) != "streaming" {
./pkg/strategy/controller.go: if strings.EqualFold(jaeger.Spec.Storage.Type, "memory") && strings.ToLower(jaeger.Spec.Strategy) != "allinone" {
./pkg/strategy/controller.go: if strings.ToLower(typ) == k {
Once they are changed, this is ready to be merged. Thanks for your PR!
Reviewed 9 of 9 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @tothandras)
vendor/github.com/PuerkitoBio/purell/purell.go, line 273 at r1 (raw file):
func forceHTTP(u *url.URL) { if strings.EqualFold(u.Scheme, "https") {
This one should probably not be here :-) (vendor)
pkg/cronjob/spark_dependencies.go
Outdated
| @@ -17,7 +17,13 @@ import ( | |||
| var supportedStorageTypes = map[string]bool{"elasticsearch": true, "cassandra": true} | |||
|
|
|||
| func SupportedStorage(storage string) bool { | |||
| return supportedStorageTypes[strings.ToLower(storage)] | |||
There was a problem hiding this comment.
@jpkrohling codecov is not happy about this change.
There was a problem hiding this comment.
Reviewed 3 of 4 files at r2, 1 of 1 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @tothandras)
pkg/cronjob/spark_dependencies.go, line 20 at r3 (raw file):
func SupportedStorage(storage string) bool { for storageType, supported := range supportedStorageTypes {
Cool, I guess you found one place where it should indeed be ToLower ;)
Signed-off-by: András Tóth <andras.toth@banzaicloud.com>
|
pkg/cronjob/spark_dependencies.go, line 20 at r3 (raw file): Previously, jpkrohling (Juraci Paixão Kröhling) wrote…
okay, reverted |
There was a problem hiding this comment.
Reviewed 1 of 1 files at r4.
Reviewable status:complete! all files reviewed, all discussions resolved
|
Thanks! |
Closes #181
Signed-off-by: András Tóth andras.toth@banzaicloud.com