Code review comment for lp:~themue/juju-core/012-local-storage

Revision history for this message
William Reade (fwereade) wrote :

LGTM with the AttemptStrategy stuff dropped.

https://codereview.appspot.com/7346052/diff/12001/environs/local/storage_test.go
File environs/local/storage_test.go (right):

https://codereview.appspot.com/7346052/diff/12001/environs/local/storage_test.go#newcode18
environs/local/storage_test.go:18: var noRetry =
trivial.AttemptStrategy{}
There's no need for an AttemptStrategy here.

https://codereview.appspot.com/7346052/diff/12001/environs/local/storage_test.go#newcode22
environs/local/storage_test.go:22: // be removed again.
These tests should be written properly, and should not be earmarked for
deletion. Do we really write these sorts of tests against an *Environ*
in the other cases? If so, what have we been smoking?

https://codereview.appspot.com/7346052/diff/12001/environs/local/storage_test.go#newcode85
environs/local/storage_test.go:85: func checkFileDoesNotExist(c *C,
storage environs.StorageReader, name string, attempt
trivial.AttemptStrategy) {
AttemptStrategy not needed.

https://codereview.appspot.com/7346052/diff/12001/environs/local/storage_test.go#newcode99
environs/local/storage_test.go:99: func checkFileHasContents(c *C,
storage environs.StorageReader, name string, contents []byte, attempt
trivial.AttemptStrategy) {
AttemptStrategy not needed.

https://codereview.appspot.com/7346052/

« Back to merge proposal