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

Revision history for this message
Tim Penhey (thumper) wrote :

https://codereview.appspot.com/13727043/diff/7001/environs/filestorage/filestorage.go
File environs/filestorage/filestorage.go (right):

https://codereview.appspot.com/13727043/diff/7001/environs/filestorage/filestorage.go#newcode107
environs/filestorage/filestorage.go:107: if err := os.MkdirAll(tmpdir,
0755); err != nil && !os.IsExist(err) {
Do you ever remove the tmpdir?

https://codereview.appspot.com/13727043/diff/7001/environs/filestorage/filestorage_test.go
File environs/filestorage/filestorage_test.go (right):

https://codereview.appspot.com/13727043/diff/7001/environs/filestorage/filestorage_test.go#newcode39
environs/filestorage/filestorage_test.go:39: s.writer, err =
filestorage.NewFileStorageWriter(s.dir, "")
how about a constant in filestorage
   const UseDefaultTmpDir = ""

then

s.writer, err = filestorage.NewFileStorageWriter(s.dir,
filestorage.UseDefaultTmpDir)

It means I don't have to go and read NewFileStorageWriter to work out
what "" means.

https://codereview.appspot.com/13727043/

« Back to merge proposal