Code review comment for lp:~dimitern/juju-core/330-general-improvements

review so far.

https://codereview.appspot.com/72860045/diff/2/environs/bootstrap/state.go
File environs/bootstrap/state.go (right):

https://codereview.appspot.com/72860045/diff/2/environs/bootstrap/state.go#newcode43
environs/bootstrap/state.go:43: logger.Debugf("putting %q to boostrap
storage %#v", StateFile, storage)
I'd be tempted lose this line entirely - it would be nicer to have
better debugging on the storage put methods. But if we keep it, I'd lose
the %#v - it could be very noisy. %T might be better.

https://codereview.appspot.com/72860045/diff/2/environs/configstore/disk.go
File environs/configstore/disk.go (right):

https://codereview.appspot.com/72860045/diff/2/environs/configstore/disk.go#newcode29
environs/configstore/disk.go:29: // Exists returns if the default
disk-based environment config storage
On 2014/03/11 04:43:22, axw wrote:
> returns true iff?

The canonical form is:

// Exists reports whether ...

I'm having difficulty understanding why this function exists though.
For a start, the name "Exists" doesn't imply in which configstore
implementation the environment info exists.

The only place that it's used in the production code, it seems that it
could be trivially replaced with a call to ReadInfo.

It's easy to "mock" the existence of the environ info without resorting
to replacing this function - just create the info. That way our tests
don't become unnecessarily dependent on internal details of the code.

I suggest removing this functon.

https://codereview.appspot.com/72860045/