state: make Addresses use Machine.Addresses

state: make Addresses use Machine.Addresses

... and APIAddresses likewise.

This requires changing some tests so that there is
actually a machine that pretends to be the state server.

https://code.launchpad.net/~rogpeppe/juju-core/451-state-addresses/+merge/191686

(do not edit description out of merge proposal)

[rog, 2013-10-17 17:58:40]

Please take a look.

[natefinch, 2013-10-18 13:07:33]

LGTM with a couple very minor nitpicks.

https://codereview.appspot.com/14619045/diff/4001/state/apiserver/common/addr...
File state/apiserver/common/addresses.go (right):

https://codereview.appspot.com/14619045/diff/4001/state/apiserver/common/addr...
state/apiserver/common/addresses.go:12: // It is implemented by state.State.
Not a fan of having the comment on the interface tell you what implements the
interface. It's way too far from the implementation, and therefor easy for it to
get out of date.  Also, it just seems contrary the idea of implicit
implementations.

https://codereview.appspot.com/14619045/diff/4001/state/apiserver/common/addr...
state/apiserver/common/addresses.go:23: st AddressAndCertGetter
st is a terrible name for an AddressAndCertGetter.  I get that the only thing
that implements it is state, but that's information external to the type, and
makes the code in this file more difficult to read.

https://codereview.appspot.com/14619045/diff/4001/state/apiserver/common/addr...
state/apiserver/common/addresses.go:28: func NewAddresser(st
AddressAndCertGetter) *Addresser {
as above

https://codereview.appspot.com/14619045/diff/4001/state/apiserver/provisioner...
File state/apiserver/provisioner/provisioner_test.go (right):

https://codereview.appspot.com/14619045/diff/4001/state/apiserver/provisioner...
state/apiserver/provisioner/provisioner_test.go:50: for i := 1; i < 3; i++ {
can you just make this 0 to 2 instead of 1 to 3?  confuses the heck out of me
as-is.

[rog, 2013-10-18 14:09:05]

Please take a look.

https://codereview.appspot.com/14619045/diff/4001/state/apiserver/common/addr...
File state/apiserver/common/addresses.go (right):

https://codereview.appspot.com/14619045/diff/4001/state/apiserver/common/addr...
state/apiserver/common/addresses.go:12: // It is implemented by state.State.
On 2013/10/18 13:07:33, nate.finch wrote:
> Not a fan of having the comment on the interface tell you what implements the
> interface. It's way too far from the implementation, and therefor easy for it
to
> get out of date.  Also, it just seems contrary the idea of implicit
> implementations.

Done.

https://codereview.appspot.com/14619045/diff/4001/state/apiserver/common/addr...
state/apiserver/common/addresses.go:23: st AddressAndCertGetter
On 2013/10/18 13:07:33, nate.finch wrote:
> st is a terrible name for an AddressAndCertGetter.  I get that the only thing
> that implements it is state, but that's information external to the type, and
> makes the code in this file more difficult to read.

Done.

https://codereview.appspot.com/14619045/diff/4001/state/apiserver/common/addr...
state/apiserver/common/addresses.go:28: func NewAddresser(st
AddressAndCertGetter) *Addresser {
On 2013/10/18 13:07:33, nate.finch wrote:
> as above

Done.

https://codereview.appspot.com/14619045/diff/4001/state/apiserver/provisioner...
File state/apiserver/provisioner/provisioner_test.go (right):

https://codereview.appspot.com/14619045/diff/4001/state/apiserver/provisioner...
state/apiserver/provisioner/provisioner_test.go:50: for i := 1; i < 3; i++ {
On 2013/10/18 13:07:33, nate.finch wrote:
> can you just make this 0 to 2 instead of 1 to 3?  confuses the heck out of me
> as-is.

Done.

[rog, 2013-10-31 12:13:32]

Please take a look.