Code review comment for lp://staging/~dave-cheney/pyjuju/go-state-machine-string

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

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.

https://codereview.appspot.com/6257072/

« Back to merge proposal