Code review comment for lp:~jameinel/juju-core/api-use-register-standard-facade

Revision history for this message
John A Meinel (jameinel) wrote :

Please take a look.

https://codereview.appspot.com/100460045/diff/20001/state/apiserver/common/registry_test.go
File state/apiserver/common/registry_test.go (right):

https://codereview.appspot.com/100460045/diff/20001/state/apiserver/common/registry_test.go#newcode198
state/apiserver/common/registry_test.go:198: // Entity ?
On 2014/05/16 10:57:24, fwereade wrote:
> Entity skipped because no facade is specific to a single entity?

All this got ripped out in the next branch, so it doesn't apply anymore.

https://codereview.appspot.com/100460045/diff/20001/state/apiserver/root.go
File state/apiserver/root.go (right):

https://codereview.appspot.com/100460045/diff/20001/state/apiserver/root.go#newcode62
state/apiserver/root.go:62: // the other ones being created on-demand.
Why is that?
On 2014/05/16 10:57:24, fwereade wrote:
> I'd guess because dataDir.

Yeah, this got fixed as well.

https://codereview.appspot.com/100460045/diff/20001/state/apiserver/upgrader/upgrader.go
File state/apiserver/upgrader/upgrader.go (right):

https://codereview.appspot.com/100460045/diff/20001/state/apiserver/upgrader/upgrader.go#newcode28
state/apiserver/upgrader/upgrader.go:28: // returned depends on who is
calling.
On 2014/05/16 10:57:24, fwereade wrote:
> We could always move the dispatching down a layer into the Upgrader
itself, if
> you consider this to be too icky.

So it is a bit odd, but since both conform to exactly the same API it is
fine. There is a small problem because of late reflection that we might
expose more of an API than we realize. I may have to come back to this
later.

https://codereview.appspot.com/100460045/

« Back to merge proposal