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

Revision history for this message
Martin Packman (gz) wrote :

LGTM.

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

https://codereview.appspot.com/98260043/diff/60001/state/action.go#newcode8
state/action.go:8: Payload map[string]interface{}
I think I'd comment these members. Clarifies thinks like what defines
the action name, and what determines the payload type.

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

https://codereview.appspot.com/98260043/diff/60001/state/unit_test.go#newcode1328
state/unit_test.go:1328: c.Assert(err, gc.NotNil)
Probably want to ErrorMatches this even though it's from below the
AddAction function.

https://codereview.appspot.com/98260043/diff/60001/state/unit_test.go#newcode1349
state/unit_test.go:1349: //TODO(jcw4): need to figure out how to inject
'contention' into the transaction
This is where we really want a generic contention helper so each add
doesn't have to futz around with testing its loop. I think William has
hopes for that at some point later.

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

« Back to merge proposal