Code review comment for lp:~themue/juju-core/go-state-unit-status

Revision history for this message
William Reade (fwereade) wrote :

>
> https://codereview.appspot.com/6454113/diff/3001/state/unit.go
> File state/unit.go (right):
>
> https://codereview.appspot.com/6454113/diff/3001/state/unit.go#newcode160
> state/unit.go:160: // Status returns the status of the unit.
> On 2012/08/08 07:57:00, fwereade wrote:
> > I don't *think* these should be constants, because the set of *-error
> states is
> > not predictable.
>
> The status not being predictable is quite a bummer. Also, the
> documentation for this method looks *very* much like the documentation
> for a bunch of constants that ought to be properly declared in the code
> as such.
>
> Can we fix this now?
>
> What if we had a single "error" status code for errors, and an
> additional info field regarding why it's broken?
>
> We've already agreed in the sprint that any hook errors will freeze the
> unit, and also agreed to save more details of erroring hooks, so all
> seems to be going in a good direction in those terms.
>
> Comments?
>
> https://codereview.appspot.com/6454113/

SGTM -- I hadn't been sure we'd actually agreed to save more hook info in the short term. We should be careful to keep existing fields in juju status output consistent with python.

« Back to merge proposal