Code review comment for lp:~johnweldon4/juju-core/action
Revision history for this message
| John Weldon (johnweldon4) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
bbfa235
(Get the code!)
Okay... addressed all
Running the tests and then I'll lbox propose
https:/
File state/action.go (right):
https:/
state/action.go:6: Id string `bson:"_id"`
On 2014/05/22 08:02:13, fwereade wrote:
> blank lines before comments please
Done.
https:/
state/action.go:9: // action to actually perform
On 2014/05/22 08:02:13, fwereade wrote:
> Name identifies the action; it should match an action defined by the
unit's
> charm.
> ?
Done.
https:/
state/action.go:12: // when this action is being performed.
On 2014/05/22 08:02:13, fwereade wrote:
> Payload holds the action's parameters, if any; it should validate
against the
> schema defined by the named action in the unit's charm.
> ?
Done.
https:/
File state/unit.go (right):
https:/
state/unit.go:1344: return newActionID, err
On 2014/05/22 08:02:13, fwereade wrote:
> ah, dammit. We shouldn't really return newActionID if err != nil.
`switch err :=
> ...; err {`?
Done.
https:/
File state/unit_test.go (right):
https:/
state/unit_
On 2014/05/22 08:02:14, fwereade wrote:
> hey, can we move these to a new suite in action_test.go? unit_test is
already
> too damn big IMO.
Done.
https:/
state/unit_
"^u#"+s.
On 2014/05/22 08:02:14, fwereade wrote:
> how about an assertSaneActionId? You can use it below where you're
currently
> discarding the new ids.
Done. but only applies to the places where we don't expect an error...
https:/
state/unit_
TestAddActionFa
On 2014/05/22 08:02:13, fwereade wrote:
> TestAddActionLi
Done.
https:/
state/unit_
TestAddActionFa
On 2014/05/22 08:02:14, fwereade wrote:
> I'd just drop this.
Done.
https:/