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
LGTM overall
https:/ /codereview. appspot. com/10166044/ diff/1/ state/apiserver /client_ test.go /client_ test.go (right):
File state/apiserver
https:/ /codereview. appspot. com/10166044/ diff/1/ state/apiserver /client_ test.go# newcode305 /client_ test.go: 305: withMockCharmSt ore(func( store MockCharmStore) {
state/apiserver
*coretesting.
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 tore' 2 times, I think I agree. We have the pattern:
'withMockCharmS
defer MakeFakeHome( ).Restore( )
elsewhere, but I guess you need the return parameter?
https:/ /codereview. appspot. com/10166044/