Code review comment for lp:~axwalk/juju-core/sshstorage-tmpdir

Revision history for this message
Andrew Wilkins (axwalk) wrote :

https://codereview.appspot.com/13660047/diff/7001/environs/sshstorage/storage_test.go
File environs/sshstorage/storage_test.go (right):

https://codereview.appspot.com/13660047/diff/7001/environs/sshstorage/storage_test.go#newcode4
environs/sshstorage/storage_test.go:4: package sshstorage
On 2013/09/18 21:30:56, thumper wrote:
> Normally the tests would be in sshstorage_test package.

> Is there any reason why this is a whitebox test?

Yes, so the tests can mock out "sshCommand". See the call to jc.Set in
SetUpSuite.

https://codereview.appspot.com/13660047/diff/7001/environs/sshstorage/storage_test.go#newcode74
environs/sshstorage/storage_test.go:74: func (s *storageSuite)
makeStorage(c *gc.C) (storage *SSHStorage, storageDir string) {
On 2013/09/18 21:30:56, thumper wrote:
> Why name the return parameters here? You aren't really gaining much.
Is it just
> to provide more assistance to the reader?

Yes, that was my rationale; I don't want to have to look at the
implementation to know what the result is.

It could be a doc comment, but in this case a self-documenting named
result is an easy win.

https://codereview.appspot.com/13660047/

« Back to merge proposal