Fix GroupsAsSubDomains option for Mesos and Marathon #868
Conversation
| sort.Sort(sort.Reverse(sort.StringSlice(splitedName))) | ||
| for i, j := 0, len(splitedName)-1; i < j; i, j = i+1, j-1 { | ||
| splitedName[i], splitedName[j] = splitedName[j], splitedName[i] | ||
| } |
There was a problem hiding this comment.
How is this different from sort.Reverse(sort.StringSlice(splitedName)) ?
There was a problem hiding this comment.
sort.Reverse returns the list in reverse sorted order, not reverse order, per the unit tests added.
There was a problem hiding this comment.
In fact no, sort.Reverse(...) is just a wrapper and does nothing ^^
I would have preferred to get this from std lib but meh ;)
There was a problem hiding this comment.
Sure. Reverse's Less implementation just reverses the Less implementation of the underlying type. So when we call Sort, it still returns in sorted order. If you have a suggested way to do this using the stdlib, I'd be happy to make that change.
There was a problem hiding this comment.
If you have a suggested way to do this using the stdlib, I'd be happy to make that change.
Sadly, I don't either ^^
|
Maybe we could get this into v1.1.1. Could you change the base of this PR to branch v1.1? |
|
Included in v1.1.1 would be great. I've had to deploy a custom build in our production environment. |
ryanleary
force-pushed
the
867-fix-subdomain
branch
from
d610b32
to
41ca450
Compare
|
Do I have to do something to retrigger the CI? Seems stuck. |
|
@ryanleary Indeed, can you force push maybe ? |
|
Pulled the reverse code into a function and pushed. |
|
LGTM 👍 |
1 similar comment
|
LGTM 👍 |
* Fix GroupsAsSubDomains option for Mesos and Marathon * Refactor reverseStringSlice function
ldez
added
area/provider/marathon
area/provider/mesos
and removed
status/2-needs-review
labels
Bugfix and new tests for the GroupsAsSubDomains bug for Mesos and Marathon providers. Closes #867.