Code review comment for lp:~thumper/juju-core/local-sudo-caller

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

LGTM

https://codereview.appspot.com/11321043/diff/1/environs/local/config.go
File environs/local/config.go (right):

https://codereview.appspot.com/11321043/diff/1/environs/local/config.go#newcode16
environs/local/config.go:16: var rootCheckFunction = func() bool {
On 2013/07/16 03:56:35, wallyworld wrote:
> Perhaps checkRoot() is better?

checkIfRoot ?

https://codereview.appspot.com/11321043/diff/1/environs/local/config_test.go
File environs/local/config_test.go (right):

https://codereview.appspot.com/11321043/diff/1/environs/local/config_test.go#newcode87
environs/local/config_test.go:87: defer
local.SetRootCheckFunction(rootCheck)
I would probably label this origRootCheck or something to that effect,
so it is immediately clear that this was the value before setting it
without having to dig up the return value of SetRootCheckFunction.

https://codereview.appspot.com/11321043/diff/1/environs/local/export_test.go
File environs/local/export_test.go (right):

https://codereview.appspot.com/11321043/diff/1/environs/local/export_test.go#newcode17
environs/local/export_test.go:17: }
This is fine as is, though for testing stuff I do still prefer the style
of

return func() {
  rootCheckFunction = old
}

But if you ever want to inspect what the real root check does, I suppose
this is fine, too.

https://codereview.appspot.com/11321043/

« Back to merge proposal