state: add Machine.String()

state: add Machine.String()

This proposal allows state.Machine to fulfil the Stringer 
interface so it is capable of printing itself in a way that
is not dependant on it's internal represntation.

https://code.launchpad.net/~dave-cheney/juju/go-state-machine-string/+merge/108077

(do not edit description out of merge proposal)

[dave_cheney.net, 2012-05-30 23:50:46]

Please take a look.

[dave_cheney.net, 2012-05-30 23:53:21]

Please take a look.

[dave_cheney.net, 2012-06-01 06:49:21]

Please take a look.

[fwereade, 2012-06-01 08:42:51]

LGTM

[niemeyer, 2012-06-05 18:45:15]

LGTM assuming you're happy with the following suggestions.

https://codereview.appspot.com/6257072/diff/6001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/6257072/diff/6001/state/machine.go#newcode38
state/machine.go:38: return fmt.Errorf("state: waiting for agent of %s: %v", m,
err)
s/state://
s/of /of machine/

https://codereview.appspot.com/6257072/diff/6001/state/machine.go#newcode80
state/machine.go:80: return fmt.Sprintf("machine %d", m.Id())
I think this should be something like:

    return strconv.Itoa(m.Id())

We already have other String methods returning identifiers, and they do not
qualify the value type. We either have to change all, or follow with the
convention.

Form another angle, it's not usual to qualify in that way, as rather than an
identifier this is an excerpt which enforces a way to construct the sentence.
Imagine writing a sentence like "can't connect machines %s and %s", for example.

[dave_cheney.net, 2012-06-05 22:55:55]

Please take a look.

https://codereview.appspot.com/6257072/diff/6001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/6257072/diff/6001/state/machine.go#newcode38
state/machine.go:38: return fmt.Errorf("state: waiting for agent of %s: %v", m,
err)
On 2012/06/05 18:45:15, niemeyer wrote:
> s/state://
> s/of /of machine/

Done.

https://codereview.appspot.com/6257072/diff/6001/state/machine.go#newcode80
state/machine.go:80: return fmt.Sprintf("machine %d", m.Id())
Done. I fixed the test to not require this form, now machine.String returns it's
id as a string, as expected.

[niemeyer, 2012-06-06 01:21:53]

You mentioned changes to the tests, but there's nothing in this branch about
that. What's here still LGTM.

[dave_cheney.net, 2012-06-06 01:26:37]

It's actually the format of the error in WaitAgentAlive, which I've fixed so the
test doesn't need to change. 

On 06/06/2012, at 11:21, n13m3y3r@gmail.com wrote:

> You mentioned changes to the tests, but there's nothing in this branch
> about that. What's here still LGTM.
> 
> https://codereview.appspot.com/6257072/

[dave_cheney.net, 2012-06-06 02:05:40]

*** Submitted:

state: add Machine.String()

This proposal allows state.Machine to fulfil the Stringer 
interface so it is capable of printing itself in a way that
is not dependant on it's internal represntation.

R=fwereade, niemeyer
CC=
https://codereview.appspot.com/6257072