Code review comment for lp:~johnweldon4/juju-core/action

Looking good. Ping me when you're on, and we can go through the
transaction loop stuff.

https://codereview.appspot.com/98260043/diff/20001/state/action.go
File state/action.go (right):

https://codereview.appspot.com/98260043/diff/20001/state/action.go#newcode8
state/action.go:8: Unit string
Drop the Unit field.

https://codereview.appspot.com/98260043/diff/20001/state/action.go#newcode31
state/action.go:31: // Id returns the mongo Id of the Action
even if it *is* the mongo id, we shouldn't *tell* people it is (if you
see what I mean).

I think that what we end up exposing to the user will want to be some
long hex string (identifiable by first few characters -- think git
revisions)... but let's not quibble about that now. This is the PK we
want, and until we expose the functionality we're free to fix it later.

https://codereview.appspot.com/98260043/diff/20001/state/unit.go
File state/unit.go (right):

https://codereview.appspot.com/98260043/diff/20001/state/unit.go#newcode1314
state/unit.go:1314: func (u *Unit) AddAction(name string, payload
map[string]interface{}) (*Action, error) {
is it an interface{}, or a map[string]interface{}? I think it's the
latter, but then the doc needs to match as well.

https://codereview.appspot.com/98260043/diff/20001/state/unit.go#newcode1332
state/unit.go:1332: // TODO(jcw4) transaction loop
also needs assert on unit life in the loop.

https://codereview.appspot.com/98260043/diff/20001/state/unit_test.go
File state/unit_test.go (right):

https://codereview.appspot.com/98260043/diff/20001/state/unit_test.go#newcode1299
state/unit_test.go:1299: c.Assert(action.Id(), gc.Matches,
"^u#"+s.unit.Name()+"#\\d+")
I'd drop the empty TokenTest in the other file and just add a couple of
bits here: check that you can get the action by Id, and that the objects
match.

https://codereview.appspot.com/98260043/