Code review comment for lp:~fwereade/juju-core/config-7-deploy-sensible-layering

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

LGTM overall

https://codereview.appspot.com/10166044/diff/1/state/apiserver/client_test.go
File state/apiserver/client_test.go (right):

https://codereview.appspot.com/10166044/diff/1/state/apiserver/client_test.go#newcode305
state/apiserver/client_test.go:305: withMockCharmStore(func(store
*coretesting.MockCharmStore) {
On 2013/06/11 13:22:07, rog wrote:
> tbh i think i'd prefer:

> store, restore := newMockCharmStore()
> defer restore()

> and lose the extra indentation.

Given I don't think you'll ever want to have a test call
'withMockCharmStore' 2 times, I think I agree. We have the pattern:

defer MakeFakeHome().Restore()

elsewhere, but I guess you need the return parameter?

https://codereview.appspot.com/10166044/

« Back to merge proposal