- Notifications
You must be signed in to change notification settings - Fork1k
chore(vpn/tunnel): sort outgoing fqdns by length#16520
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
| fqdn=append(fqdn,name.WithTrailingDot()) | ||
| } | ||
| sort.Slice(fqdn,func(i,jint)bool { | ||
| returnlen(fqdn[i])<len(fqdn[j]) |
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.
Wouldn't a better sort be by the amount of periods?
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.
Not necessarily? If we have:
dev.wsname.ethan.coderdev.wsname.me.coderI'd want the latter to be shown. I guess we could do number of periods AND length?
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.
Actually, I don't think it's worth doing that. We want the shortest name because that's what looks the best in the UI. With all the hostnames we set currently, the shortest name is always going to have the least periods. If we ever change that, we can revisit this.
0e728a7 intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
On the Mac app, we want to display the shortest FQDN - we might as well do the sorting as they leave the tunnel.
Right now it's coming from a map, so it's they arrive in a random order each peer update.