state: rationalize destruction APIs

state: rationalize destruction APIs

EnsureDying has become Destroy, to match Service and Relation;
State.RemoveMachine has become Machine.Remove, as has Service.RemoveUnit
become Unit.Remove. This change is entirely mechanical.

https://code.launchpad.net/~fwereade/juju-core/state-death-api/+merge/143555

Requires: https://code.launchpad.net/~fwereade/juju-core/state-service-api/+merge/143394

(do not edit description out of merge proposal)

[fwereade, 2013-01-16 17:36:04]

Please take a look.

[fwereade, 2013-01-18 07:57:39]

Please take a look.

[dimitern, 2013-01-22 14:14:55]

LGTM overall, just some suggestions.

https://codereview.appspot.com/7142043/diff/3001/state/relation.go
File state/relation.go (right):

https://codereview.appspot.com/7142043/diff/3001/state/relation.go#newcode156
state/relation.go:156: // removeOps returns the operations necessary to remove
the relation. If
This is much clearer now, thanks for the comments!

https://codereview.appspot.com/7142043/diff/3001/state/relation_test.go
File state/relation_test.go (right):

https://codereview.appspot.com/7142043/diff/3001/state/relation_test.go#newco...
state/relation_test.go:821: err = pru.EnterScope(nil) // to create the
subordinate
It seems we have a bunch of these Enter/LeaveScope pairs working on a principal
unit (P). Maybe it's a good candidate for a P.RefreshScope or
P.UpdateSubordinates or something like that. Possibly also a separate helper
func...(P). That way the explanation why this happens can be in a single
location and the whole operation will be obvious. What do you think?

https://codereview.appspot.com/7142043/diff/3001/state/unit_test.go
File state/unit_test.go (right):

https://codereview.appspot.com/7142043/diff/3001/state/unit_test.go#newcode177
state/unit_test.go:177: _, err := s.State.AddService("logging", subCharm)
See below

https://codereview.appspot.com/7142043/diff/3001/state/unit_test.go#newcode414
state/unit_test.go:414: for i := 0; i < 2; i++ {
See below

https://codereview.appspot.com/7142043/diff/3001/state/unit_test.go#newcode541
state/unit_test.go:541: name := "logging" + strconv.Itoa(i)
At these places the removal of AddUnitSubordinateTo makes the things less
obvious. Maybe it's good to have a comment before each of these, just to clarify
what's going on.

[rog, 2013-01-25 16:49:57]

LGTM. very nice.

[fwereade, 2013-01-25 17:49:16]

*** Submitted:

state: rationalize destruction APIs

EnsureDying has become Destroy, to match Service and Relation;
State.RemoveMachine has become Machine.Remove, as has Service.RemoveUnit
become Unit.Remove. This change is entirely mechanical.

R=dimitern, rog
CC=
https://codereview.appspot.com/7142043

https://codereview.appspot.com/7142043/diff/3001/state/relation_test.go
File state/relation_test.go (right):

https://codereview.appspot.com/7142043/diff/3001/state/relation_test.go#newco...
state/relation_test.go:821: err = pru.EnterScope(nil) // to create the
subordinate
On 2013/01/22 14:14:55, dimitern wrote:
> It seems we have a bunch of these Enter/LeaveScope pairs working on a
principal
> unit (P). Maybe it's a good candidate for a P.RefreshScope or
> P.UpdateSubordinates or something like that. Possibly also a separate helper
> func...(P). That way the explanation why this happens can be in a single
> location and the whole operation will be obvious. What do you think?

I *think* this is the only place we do an Enter/Leave -- it's very specific to
this situation, which serves tests that expects both that the subordinate exists
and that scope is empty to begin with.

So, in this case, -1; in general I'm quite keen on the idea of an improved
vocabulary for state testing, but I'm holding off on it because: if I do it
piecemeal, I won't get it finished anytime soon -- I *still* haven't finally
killed AUST; and if I do it all at once nobody will be able to bear reviewing it
;).

https://codereview.appspot.com/7142043/diff/3001/state/unit_test.go
File state/unit_test.go (right):

https://codereview.appspot.com/7142043/diff/3001/state/unit_test.go#newcode177
state/unit_test.go:177: _, err := s.State.AddService("logging", subCharm)
On 2013/01/22 14:14:55, dimitern wrote:
> See below

Done.

https://codereview.appspot.com/7142043/diff/3001/state/unit_test.go#newcode414
state/unit_test.go:414: for i := 0; i < 2; i++ {
On 2013/01/22 14:14:55, dimitern wrote:
> See below

Done.