better-arranged charm manager

better-arranged charm manager

I think it's looking nicer now; I'm not convinced that adding a 3rd uniter
state will make things any simpler or clearer, and I feel the cbSuccess
mechanism in Manager.Update actually works pretty well. Thoughts?

https://code.launchpad.net/~fwereade/juju-core/working-charm-manager/+merge/122281

(do not edit description out of merge proposal)

[niemeyer, 2012-08-31 14:29:11]

https://codereview.appspot.com/6488062/diff/1/worker/uniter/charm/charm.go
File worker/uniter/charm/charm.go (right):

https://codereview.appspot.com/6488062/diff/1/worker/uniter/charm/charm.go#ne...
worker/uniter/charm/charm.go:52: updatesPath := filepath.Join(stateDir,
"charm-updates")
As I was reading this it occurred to me: instead of having stateDir, could we
possibly put things under .git? It's not a big deal, but it might simplify a bit
the interface and make the directory self-contained.

https://codereview.appspot.com/6488062/diff/1/worker/uniter/charm/charm.go#ne...
worker/uniter/charm/charm.go:62: // DeployPath returns the path to the directory
in which the charm is deployed.
Why do we call it charmDir on one end and DeployPath on the other?

https://codereview.appspot.com/6488062/diff/1/worker/uniter/charm/charm.go#ne...
worker/uniter/charm/charm.go:147: if err = cbSuccess(); err != nil {
I don't get the idea here. Update is managing its own state. We ask it to
Update, it updates, and tracks the fact it updates. Success means this method
returned without errors. The fact we're hooking into this feels like a hack.

I still think the previous commentary made around the status logic is sensible,
and is why you've been fighting with the current API. If you disagree, I'd
appreciate understanding your point of view.