Code review comment for lp:~vila/bzr/525571-minimal-atomic-write-bazaar-conf-files-2.1

Revision history for this message
Vincent Ladeuil (vila) wrote :

> 1) If we know we are accessing a local file (since you are using open()
> directly), then we could just use AtomicFile directly.

Done.

>
> 2) I'm a bit concerned about the need for
> safe_unicode(self._get_filename()).

This was triggered in my cleanup branch by a test in a unicode home dir, Ill look into it again there.

>
> 3) You can do
>
> dirname, basename = osutils.split(fname)
>
> rather than doing it 2x.

1) I was aiming minimal, 2) different encodings were needed.

« Back to merge proposal