-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Split(".www.miek.nl.") -> []string{"", "www", "miek", "nl"}, #306
Comments
Change the interface, add a new func or make a note of it for a someday-maybe |
We could return |
Maybe a simple breaking change log could make things like this easier. A low effort way to do it would be to just put '#breaking` or similar in commit messages and then extract into the readme a list of commits that break things. This would be even more useful for changes that silently break things like #281 seems to have done to rrda (see fcambus/rrda#11). |
[ Quoting notifications@github.com in "Re: [dns] Split(".www.miek.nl.") ->..." ]
I agree. Should still be careful, but bugs need to be fixed even though that could I also like the '#breaking' idea. /Miek Miek Gieben |
Sketched up a way it could work. |
Coming back to this specific issue and looking through other label function, such as CountLabel for instance, they don't return a value plus error. So either we break them all, or we assume and document that in the input should be a properly formatted dns name/label. I'm leaning towards the latter (garbage in, garbage out) |
That sounds okay. If there's demand in the future error returning variants could always go in |
[ Quoting notifications@github.com in "Re: [dns] Split(".www.miek.nl.") ->..." ]
Ok, let's agree to that. They are some prinln("deprecated") littered in the /Miek Miek Gieben |
This might be a bug (or not). It's is basically an illegal domain name that you then split...
The text was updated successfully, but these errors were encountered: