The name "Address" is used inconsistently
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
juju-core |
Won't Fix
|
Low
|
Unassigned |
Bug Description
[16:43:53] <rogpeppe> mgz: there's a bit of confusion between hostname:port and hostname only in our code. i wonder if we should consistently name the former "Address" (fitting in with the "address" as passed to net.Dial) and the latter "HostName"
[16:45:24] <rogpeppe> mgz: as it is, we've got net.Dial and now replicaset.
[16:45:54] <mgz> rogpeppe: that sounds sensible
[16:46:22] <mgz> well, isn't there a better term...
[16:46:27] * mgz goes to look at rfcs
[16:46:34] <rogpeppe> mgz: quite possibly
[16:47:16] <rogpeppe> mgz: net.Dial calls it "host"
[16:47:32] <mgz> 'authority' would be correct for the combo
[16:47:53] <mgz> [ userinfo "@" ] host [ ":" port ]
[16:47:57] <rogpeppe> mgz: maybe somehow "instance.HostName" seems a better fit than "instance.Host" to me though
[16:48:37] <rogpeppe> mgz: i think we should go with the Go convention of "address" for host:port
[16:49:10] <rogpeppe> mgz: it's used all over the place, and we can't change that in the Go tree
[16:49:38] <mgz> host is ipvX / ipv6 / ipv4 / reg-name
[16:49:44] <rogpeppe> mgz: alternatively we could use "HostPort"
[16:49:59] <rogpeppe> mgz: yeah
[16:50:01] <mgz> so, "registered name" where we explicitly mean not a ip address
[16:50:12] <mgz> I quite like hostport
[16:51:06] <rogpeppe> mgz: hostport is also used in go stdlib too, e.g. http://
[16:51:08] <mgz> (looking at rfc 3986 btw, there have been several url ones that somewhat update each time)
[16:51:13] <rogpeppe> mgz: but it's not so widespread
[16:52:25] <rogpeppe> mgz: maybe we *should* consistently use Host and HostPort throughout
[16:52:54] <rogpeppe> mgz: it mirrors the string concatenation nicely
[16:53:10] <mgz> that sounds like a plan
[16:53:13] <rogpeppe> mgz: and instance.Host could grow on me, i think
[16:53:41] <rogpeppe> mgz: thanks
[16:53:56] <rogpeppe> mgz: i'll file a bug, and conform to that convention in new code
Changed in juju-core: | |
status: | New → Triaged |
importance: | Undecided → Low |
tags: | added: tech-debt |
Changed in juju-core: | |
status: | Triaged → Won't Fix |