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

Revision history for this message
John Weldon (johnweldon4) wrote :

Okay proposing again.

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{}
On 2014/05/21 20:35:25, gz wrote:
> I think I'd comment these members. Clarifies thinks like what defines
the action
> name, and what determines the payload type.

Done.

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)
On 2014/05/21 20:35:25, gz wrote:
> Probably want to ErrorMatches this even though it's from below the
AddAction
> function.

Done.

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
On 2014/05/21 20:35:25, gz wrote:
> 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.

Yep

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

« Back to merge proposal