LGTM with final trivials. Sorry :(.
https://codereview.appspot.com/98260043/diff/100001/state/action.go File state/action.go (right):
https://codereview.appspot.com/98260043/diff/100001/state/action.go#newcode6 state/action.go:6: Id string `bson:"_id"` blank lines before comments please
https://codereview.appspot.com/98260043/diff/100001/state/action.go#newcode9 state/action.go:9: // action to actually perform Name identifies the action; it should match an action defined by the unit's charm.
?
https://codereview.appspot.com/98260043/diff/100001/state/action.go#newcode12 state/action.go:12: // when this action is being performed. Payload holds the action's parameters, if any; it should validate against the schema defined by the named action in the unit's charm.
https://codereview.appspot.com/98260043/diff/100001/state/unit.go File state/unit.go (right):
https://codereview.appspot.com/98260043/diff/100001/state/unit.go#newcode1344 state/unit.go:1344: return newActionID, err ah, dammit. We shouldn't really return newActionID if err != nil. `switch err := ...; err {`?
https://codereview.appspot.com/98260043/diff/100001/state/unit_test.go File state/unit_test.go (right):
https://codereview.appspot.com/98260043/diff/100001/state/unit_test.go#newcode1295 state/unit_test.go:1295: func (s *UnitSuite) TestAddAction(c *gc.C) { hey, can we move these to a new suite in action_test.go? unit_test is already too damn big IMO.
https://codereview.appspot.com/98260043/diff/100001/state/unit_test.go#newcode1311 state/unit_test.go:1311: c.Assert(action.Id(), gc.Matches, "^u#"+s.unit.Name()+"#\\d+") how about an assertSaneActionId? You can use it below where you're currently discarding the new ids.
https://codereview.appspot.com/98260043/diff/100001/state/unit_test.go#newcode1318 state/unit_test.go:1318: func (s *UnitSuite) TestAddActionFailsOnDeadUnit(c *gc.C) { TestAddActionLifecycle?
https://codereview.appspot.com/98260043/diff/100001/state/unit_test.go#newcode1357 state/unit_test.go:1357: func (s *UnitSuite) TestAddActionFailsOnExcessiveContention(c *gc.C) { I'd just drop this.
https://codereview.appspot.com/98260043/
« Back to merge proposal
LGTM with final trivials. Sorry :(.
https:/ /codereview. appspot. com/98260043/ diff/100001/ state/action. go
File state/action.go (right):
https:/ /codereview. appspot. com/98260043/ diff/100001/ state/action. go#newcode6
state/action.go:6: Id string `bson:"_id"`
blank lines before comments please
https:/ /codereview. appspot. com/98260043/ diff/100001/ state/action. go#newcode9
state/action.go:9: // action to actually perform
Name identifies the action; it should match an action defined by the
unit's charm.
?
https:/ /codereview. appspot. com/98260043/ diff/100001/ state/action. go#newcode12
state/action.go:12: // when this action is being performed.
Payload holds the action's parameters, if any; it should validate
against the schema defined by the named action in the unit's charm.
?
https:/ /codereview. appspot. com/98260043/ diff/100001/ state/unit. go
File state/unit.go (right):
https:/ /codereview. appspot. com/98260043/ diff/100001/ state/unit. go#newcode1344
state/unit.go:1344: return newActionID, err
ah, dammit. We shouldn't really return newActionID if err != nil.
`switch err := ...; err {`?
https:/ /codereview. appspot. com/98260043/ diff/100001/ state/unit_ test.go
File state/unit_test.go (right):
https:/ /codereview. appspot. com/98260043/ diff/100001/ state/unit_ test.go# newcode1295 test.go: 1295: func (s *UnitSuite) TestAddAction(c *gc.C) {
state/unit_
hey, can we move these to a new suite in action_test.go? unit_test is
already too damn big IMO.
https:/ /codereview. appspot. com/98260043/ diff/100001/ state/unit_ test.go# newcode1311 test.go: 1311: c.Assert( action. Id(), gc.Matches, unit.Name( )+"#\\d+ ")
state/unit_
"^u#"+s.
how about an assertSaneActionId? You can use it below where you're
currently discarding the new ids.
https:/ /codereview. appspot. com/98260043/ diff/100001/ state/unit_ test.go# newcode1318 test.go: 1318: func (s *UnitSuite) ilsOnDeadUnit( c *gc.C) { fecycle?
state/unit_
TestAddActionFa
TestAddActionLi
https:/ /codereview. appspot. com/98260043/ diff/100001/ state/unit_ test.go# newcode1357 test.go: 1357: func (s *UnitSuite) ilsOnExcessiveC ontention( c *gc.C) {
state/unit_
TestAddActionFa
I'd just drop this.
https:/ /codereview. appspot. com/98260043/