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

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) {
On 2013/09/18 04:09:23, thumper wrote:
> Do you ever remove the tmpdir?

No, never. I wasn't sure about this when I implemented it, but thinking
about it some more I think it should create/remove it iff
tmpdir==UseDefaultTmpDir. If a directory is explicitly specified, it
must exist and won't be removed. Then juju sync-tools won't leave tmpdir
turds.

I've updated the implementation, docstring and tests to match this.

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, "")
On 2013/09/18 04:09:23, thumper wrote:
> 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.

Good idea. Done.

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