Code review comment for lp:~mattyw/juju-core/add-service-owner-to-status

On 2014/02/27 13:47:33, fwereade wrote:
> On 2014/02/27 13:45:51, fwereade wrote:
> > On 2014/02/27 10:40:25, dimitern wrote:
> > > On 2014/02/25 10:15:52, dimitern wrote:
> > > > Looks good, but make sure you're familiar with Martin's recent
1.16
> > > > compatibility for status changes:
https://codereview.appspot.com/66590043/
> > > >
> > > > You'll need to use the FullStatus API call.
> > >
> > > Sorry, so your changes are in statecmd, which gets called
internally in
> > > apiserver for both FullStatus and Status.
> > > This means you don't need to care about FullStatus in your CL.
> > > Your changes LGTM assuming you've tested them live as well, just
in case.
> >
> > I'm fairly strongly -1 on this, because status is bloated enough
already. Can
> we
> > please start adding flags to status for additional information?

> And especially in status, adding the owner to every unit feels wrong.
If units
> ever get explicit owners we can add it then; for now, the service
feels like a
> much better place. So not lgtm without discussion, at least.

Fair enough - I was hoping to have discussion with you and roger about
this - the implementation here includes the owner as part of the service
though - not part of the unit

https://codereview.appspot.com/67870043/